The first segment was not limited to the configured maximum line length
allowing it to be up to 65k. This could result in the next input length
being negative, which while handled properly by the code, did trigger a
debug validation assertion.
The fix is to be consistent and apply the limit to the first segment as
well, which does ensure the input_len could never be less than 0.
Ticket #5281
Adds a PDU frame to the DNS parser. For UDP this is the DNS payload
portion of the DNS packet, for TCP this is the payload minus the leading
legth field.
Ticket: 4984
Wrap the calls behind frames to C code if a `cfg!(not(test))` so they
don't get compiled when running Rust unit tests. Linkage to C functions
is not yet available for Rust unit tests, and this will keep the check
out of individual parsers.
Ticket: 4984
Added sections along packet-alert-max config section explaining
packet alert queue overflow (when Suri reaches packet alert max), when
alerts are discarded etc.
Since from the user perspective it shouldn't matter how we process the
alert queue, the term "replace" is used, even though there's not exactly
a replacing action happening, with the queue bein pre-processed before
being appended to the Packet.
Also described the associated stats and added an explanation on when to
change packet-alert-max.
Task #5178
Since we now only copy the PacketAlerts to the Packet's queue after
processing them, we no longer do packet alert appending from
detect-engine-alert, nor do we remove PacketAlerts from the queue (if
they're discarded by overflow or thresholding, they're not copied to the
final alert queue).
Task #4943
The maximum of possible alerts triggered by a unique packet was
hardcoded to 15. With usage of 'noalert' rules, that limit could be
reached somewhat easily. Make that configurable via suricata.yaml.
Conf Bug#4941
Task #4207
Instead of a method that is required to return a slice of transactions,
use 2 methods, one to return the number of transactions in the
collection, and another to get a transaction by its index in the
collection.
This allows for the transaction collection to not be a contiguous array
and instead can be a VecDeque, or possibly another collection type that
supports retrieval by index.
Ticket #5278
Fuzzers found a possible integer overflow bug when parsing response
messages. To fix that, removed the case where we incremented the parsed
field length and created a new message type for situations where Suri
parsers an Unknown message. This is good because there may happen that
an unknown message to Suri is valid, and in this case, we would still be
able to log it.
Philippe Antoine found the bug while fuzzing with rust debug assertions.
Bug #5016
cppcheck:
src/util-logopenfile.c:743:13: warning: %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(threaded_name, len, "%s.%d.%s", tname, unique_id, ext);
^
src/util-logopenfile.c:752:9: warning: %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(threaded_name, len, "%s.%d", original_name, unique_id);
^
Bug: #5291.
cppcheck:
src/util-ja3.c:197:28: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
(*buffer)->used += snprintf((*buffer)->data, (*buffer)->size, "%d",
^
src/util-ja3.c:201:28: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
(*buffer)->used += snprintf((*buffer)->data + (*buffer)->used,
^
Bug: #5291.
Check is always true but confuses cppcheck:
src/log-pcap.c:1224:32: warning: Either the condition 'filename' is redundant or there is possible null pointer dereference: filename. [nullPointerRedundantCheck]
if ((pl->prefix = SCStrdup(filename)) == NULL) {
^
src/log-pcap.c:1421:9: note: Assuming that condition 'filename' is not redundant
if (filename) {
^
src/log-pcap.c:1224:32: note: Null pointer dereference
if ((pl->prefix = SCStrdup(filename)) == NULL) {
^
Bug: #5291.
cppcheck:
src/source-af-packet.c:1762:19: warning: Size of pointer 'v2' used instead of size of its data. This is likely to lead to a buffer overflow. You probably intend to write 'sizeof(*v2)'. [pointerSize]
ptv->ring.v2 = SCMalloc(ptv->req.v2.tp_frame_nr * sizeof (union thdr *));
^
src/source-af-packet.c:1767:26: warning: Size of pointer 'v2' used instead of size of its data. This is likely to lead to a buffer overflow. You probably intend to write 'sizeof(*v2)'. [pointerSize]
memset(ptv->ring.v2, 0, ptv->req.v2.tp_frame_nr * sizeof (union thdr *));
^
scan-build:
CC source-af-packet.o
source-af-packet.c:1762:24: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'union thdr *' [unix.MallocSizeof]
ptv->ring.v2 = SCMalloc(ptv->req.v2.tp_frame_nr * sizeof (union thdr *));
^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~
./util-mem.h:35:18: note: expanded from macro 'SCMalloc'
^~~~~~
1 warning generated.
Bug: #5291.
cppcheck:
src/util-time.c:255:18: warning: Either the condition 'str!=NULL' is redundant or there is possible null pointer dereference: str. [nullPointerRedundantCheck]
snprintf(str, size, "ts-error");
^
src/util-time.c:252:48: note: Assuming that condition 'str!=NULL' is not redundant
if (likely(t != NULL && fmt != NULL && str != NULL)) {
^
src/util-time.c:255:18: note: Null pointer dereference
snprintf(str, size, "ts-error");
^
Only `t` could possibly be NULL if `localtime_r` fails elsewhere.
Bug: #5291.
cppcheck:
src/detect-engine.c:3643:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
^
src/detect-engine.c:3707:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(prefix, sizeof(prefix), "multi-detect.%d.reload.%d", tenant_id, reload_cnt);
^
src/detect-engine.c:4086:17: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
^
Bug: #5291.
cppcheck:
src/util-reference-config.c:179:9: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
fd = NULL;
^
Bug: #5291.
cppcheck:
src/util-runmodes.c:210:9: warning: %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(tname, sizeof(tname), "%s#%02u", thread_name_workers, thread+1);
^
src/util-runmodes.c:211:9: warning: %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(qname, sizeof(qname), "pickup%u", thread+1);
^
src/util-runmodes.c:455:9: warning: %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(tname, sizeof(tname), "%s#%02u", thread_name_workers, thread+1);
^
src/util-runmodes.c:457:9: warning: %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(qname, sizeof(qname), "pickup%u", thread+1);
^
src/runmode-erf-file.c:188:9: warning: %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(tname, sizeof(tname), "%s#%02u", thread_name_workers, thread+1);
^
src/runmode-erf-file.c:189:9: warning: %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(qname, sizeof(qname), "pickup%u", thread+1);
^
src/runmode-pcap-file.c:201:9: warning: %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(tname, sizeof(tname), "%s#%02u", thread_name_workers, thread+1);
^
src/runmode-pcap-file.c:202:9: warning: %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'. [invalidPrintfArgType_uint]
snprintf(qname, sizeof(qname), "pickup%u", thread+1);
^
Bug: #5291.
cppcheck:
src/util-mpm-ac-ks.c:1452:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
printf("Total states in the state table: %d\n", ctx->state_count);
^
src/util-mpm-ac-ks.c:606:34: error: Signed integer overflow for expression '1<<31'. [integerOverflow]
encoded_next_state |= (1 << 31);
^
Bug: #5291.
cppcheck:
src/util-classification-config.c:189:9: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
fd = NULL;
^
Bug: #5291.
Rearrange code slightly to make it more clear that `found` cannot
be NULL further down the loop.
cppcheck:
src/detect-engine-content-inspection.c:316:50: warning: Either the condition 'found!=NULL' is redundant or there is overflow in pointer subtraction. [nullPointerArithmeticRedundantCheck]
match_offset = (uint32_t)((found - buffer) + cd->content_len);
^
src/detect-engine-content-inspection.c:308:30: note: Assuming that condition 'found!=NULL' is not redundant
} else if (found != NULL && (cd->flags & DETECT_CONTENT_NEGATED)) {
^
src/detect-engine-content-inspection.c:316:50: note: Null pointer subtraction
match_offset = (uint32_t)((found - buffer) + cd->content_len);
^
Bug: #5291.
cppcheck flagged this as:
src/detect-engine-analyzer.c:1359:13: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
fprintf(rule_engine_analysis_FD, " Rule contains %d content options, %d http content options, %d pcre options, and %d pcre options with http modifiers.\n", rule_content, rule_content_http, rule_pcre, rule_pcre_http);
^
src/detect-engine-analyzer.c:1359:13: warning: %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
fprintf(rule_engine_analysis_FD, " Rule contains %d content options, %d http content options, %d pcre options, and %d pcre options with http modifiers.\n", rule_content, rule_content_http, rule_pcre, rule_pcre_http);
^
src/detect-engine-analyzer.c:1359:13: warning: %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
fprintf(rule_engine_analysis_FD, " Rule contains %d content options, %d http content options, %d pcre options, and %d pcre options with http modifiers.\n", rule_content, rule_content_http, rule_pcre, rule_pcre_http);
^
src/detect-engine-analyzer.c:1359:13: warning: %d in format string (no. 4) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
fprintf(rule_engine_analysis_FD, " Rule contains %d content options, %d http content options, %d pcre options, and %d pcre options with http modifiers.\n", rule_content, rule_content_http, rule_pcre, rule_pcre_http);
^
Bug: #5291.
Cppcheck flagged this:
src/detect-engine-address.c:1035:48: warning: Either the condition 'ghn!=NULL' is redundant or there is possible null pointer dereference: gh. [nullPointerRedundantCheck]
int r = DetectAddressIsCompleteIPSpaceIPv4(gh->ipv4_head);
^
src/detect-engine-address.c:1297:17: note: Assuming that condition 'ghn!=NULL' is not redundant
if (ghn != NULL) {
^
src/detect-engine-address.c:1283:44: note: Calling function 'DetectAddressIsCompleteIPSpace', 1st argument 'ghn' value is 0
if (DetectAddressIsCompleteIPSpace(ghn)) {
^
src/detect-engine-address.c:1035:48: note: Null pointer dereference
int r = DetectAddressIsCompleteIPSpaceIPv4(gh->ipv4_head);
^
Cleanup code could only be reached with non-NULL pointers, so simplify checks.
Bug: #5291.
Philippe Antoine
Jason Ish
dependabot[bot]
Juliana Fajardini
Sascha Steinbiss
Victor Julien