Bug #7767
open
DetectBytetestParse: remove meaningless data_offset == NULL check
Added by Boris Tonofa 21 days ago.
Updated 8 days ago.
Affected Versions:
6.0.13,
7.0.0,
6.0.14,
7.0.1,
6.0.15,
7.0.2,
6.0.16,
7.0.3,
6.0.17,
7.0.4,
6.0.18,
6.0.19,
7.0.5,
6.0.20,
7.0.6,
7.0.7,
7.0.8,
7.0.9,
7.0.10,
8.0.0-beta1,
8.0.0-rc1,
7.0.11,
8.0.0,
7.0.12,
8.0.1,
git master
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.
- 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.
Also available in: Atom
PDF