Bug #7767
closedDetectBytetestParse: remove meaningless data_offset == NULL check
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.
Updated by Boris Tonofa about 1 month 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.
Updated by Philippe Antoine 21 days ago
- Status changed from New to Closed
- Target version changed from TBD to 8.0.0
Fixed by 66e7437befe5bff9b7a1d6246b01c8ff76252901