Project

General

Profile

Actions

Bug #7767

open

DetectBytetestParse: remove meaningless data_offset == NULL check

Added by Boris Tonofa 21 days ago. Updated 8 days ago.

Status:
New
Priority:
Normal
Assignee:
Target version:

Description

description

File / Function: src/detect/detect_bytetest.c → DetectBytetestParse()
Issue:
The code checks data_offset == NULL instead of the caller-supplied offset pointer.
If the caller passes NULL for offset, the function later dereferences that
NULL pointer and Suricata crashes:

if (data_offset[0] != '-' && isalpha((unsigned char)data_offset[0])) {
if (data_offset NULL) { /* should be: if (offset NULL) /
...
}
*offset = SCStrdup(data_offset); /
offset NULL → SIGSEGV */
}

Actual result: Suricata terminates with a segmentation fault.
Expected result: The function should return an error (NULL) and log the problem without crashing.

Proposed fix: Replace the condition with if (offset NULL); no other changes needed.

Actions #1

Updated by Boris Tonofa 8 days ago · Edited

  • Subject changed from DetectBytetestParse(): incorrect NULL check on offset pointer causes crash to DetectBytetestParse: remove meaningless data_offset == NULL check

In DetectBytetestParse the code contains

if (data_offset == NULL) {
    
}

Yet data_offset has already been validated as non-NULL only a few lines earlier, so this condition can never be true. The guard was probably meant for the offset argument, but throughout the codebase offset is never passed as NULL. (In unit tests offset is NULL, but those tests also pass value == NULL, which makes the function return before offset is dereferenced.)

The redundant check clutters the control flow and triggers false-positive static-analysis warnings. It must be removed to simplify the code and silence the analyzer.

Actions

Also available in: Atom PDF