Addresses edge case: > 4 bytes at the end of the input with 2 or more
spaces.
Changes length type for remainder processing to allow for much longer
lines, which can happen in practice.
Adds a series of debug validation checks with real error handling
as well, to assist the fuzzer to find more edge cases.
So far, it was the job of caller to send the bae64 decoder a perfect
block of data and take care of the destination buffer (decoded data)
size. Now, make it the decoder's job to take care of any space
constraints that the destination buffer may have and return accordingly.
Also, handle space characters in base64 encoded data as per RFC 2045.
Update MIME parser accordingly to handle the base64 data.
Ticket: 5315
The ideal line terminator for an SMTP line is <CRLF>. But, given that
bare LF is still allowed by many systems despite the prohibition by
standards, we have to consider that. In order to simplify things, we
consider bare CR as line terminators as well while updating the
delimiter parameter correctly if they were to be followed by a LF
immediately or as a part of next fragment.
This takes care of some edge cases that made base64 decoder error out
because unexpected data was sent to it at times.
Ticket: 5316
Move it away from http2 to generic core crate.
And use it for DCERPC (and SMB)
And remove the C version.
Main change in API is the free function is not free itself, but
a rust wrapper around unbox.
Ticket: #4112
Without dangerous snprintf pattern identified by CodeQL
even if this pattern is not a problem in those precise cases,
it may easily get copy pasted in a dangerous place, so better
get rid of it and make CodeQL happy
As input, input_len and direction only last for the scope of
one call of AppLayerParserParse, it is not necessary to keep them
in FtpState which lives longer, so we consume less memory.
Rules that look like they should be IP-only but contain a negated rule
address are now marked with an LIKE_IPONLY flag. This is so they are
treated like IPONLY rules with respect to flow action, but don't
interfere with other IPONLY processing like using the radix tree.
Ticket: #5361
Set pseudo packet datalink to the global one. This fixes the case
where the pcap handle is open with information coming from a
pseudo packet. Without this, we did end up in most cases with
an Ethernet packet being written in a Raw pcap.
As Suricata is not supporting pcap-ng we have to stick with one single
datalink type for the capture if ever we want to do pcap logging.
Assuming this, this patch introduces a function to set the link
type globally. This will be used with pcap conditional logging
to get the logging of TCP segments with the correct link type.
We were missing the first packet when using condition pcap logging
in tag mode as it was not tagged. As a result we were not getting
the stream data triggering the alert in the pcap file.
This patch adds a function to get the current pcap file name that
will be used to current packet. This patch also updates EVE
alerts to add pcap output filename when pcap capture is done in
multi or normal mode.
This patch update the alert mode of pcap logging.
It uses the packet header data added to the TCP segments
to build packets corresponding to the acked data that did trigger
the alert. It then write it to the pcap file before starting to
dump all packet for the flow that did alert.
This patch optionally adds packet header to the TCP segment
and update the for each segment function by changing the
callback.
This patch is based on the work by Scott Jordan <scottfgjordan@gmail.com>
When reloading rules, respect `--set default-rule-path=...` from the
command line if set.
Previously the rule reload would always take the default-rule-path from
the configuration file, even if overrided on the command line.
Issue: #1911
In the unlikely case of AlertQueueExpand failure, we were incrementing
the discarded alerts stats in AlertQueueAppend via the Packet member in the
DetectEngineThreadCtx, which may not be initialized yet.
Bug #5353
Do not mask protocols on both directions with only first packet
For instance :
When the first packet is no valid DNS but on port 53 (a junk request)
second packet (error response from server) does not get checked for DNS
as first packet bit masked away DNS for both directions
Ticket: #2757
kill(0, ...) re-raises the signal to every processes in the process
group which may impact unrelated processes.
Concretely, in our CI pipeline, a segfaulting Suricata process killed
the test driver.
src/detect-engine-state.c:127:91: style: Suspicious calculation. Please use parentheses to clarify the code. The code ''a&b?c:d'' should be written as either ''(a&b)?c:d'' or ''a&(b?c:d)''. [clarifyCalculation]
DetectEngineStateDirection *dir_state = &state->dir_state[direction & STREAM_TOSERVER ? 0 : 1];
^
src/detect-engine-state.c:194:53: style: Suspicious calculation. Please use parentheses to clarify the code. The code ''a&b?c:d'' should be written as either ''(a&b)?c:d'' or ''a&(b?c:d)''. [clarifyCalculation]
de_state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].filestore_cnt += file_no_match;
^
src/detect-engine-state.c:201:57: style: Suspicious calculation. Please use parentheses to clarify the code. The code ''a&b?c:d'' should be written as either ''(a&b)?c:d'' or ''a&(b?c:d)''. [clarifyCalculation]
if (de_state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].filestore_cnt == sgh->filestore_cnt)
^
Since GCC 12 the memcmp code using `_mm_blendv_epi8` failed to work.
Inspection of the disassembled objects suggests that it simply omits
the instruction on systems that are not AVX512 capable. On AVX512
it does replace it with VPCMPB logic that appears to work.
Luckily our use of blend is actually uncessary. A simple AND is sufficient.
Bug: #5312.
cppcheck:
src/util-memcmp.h:281:18: warning: Identical condition 'len-offset<16', second condition is always false [identicalConditionAfterEarlyExit]
if (diff < 16) {
^
src/util-memcmp.h:280:24: note: 'diff' is assigned value 'len-offset' here.
int diff = len - offset;
^
src/util-memcmp.h:269:33: note: If condition 'len-offset<16' is true, the function will return/exit
if (likely(len - offset < 16)) {
^
src/util-memcmp.h:281:18: note: Testing identical condition 'len-offset<16'
if (diff < 16) {
^
src/util-memcmp.h:344:18: warning: Identical condition 'len-offset<16', second condition is always false [identicalConditionAfterEarlyExit]
if (diff < 16) {
^
src/util-memcmp.h:343:24: note: 'diff' is assigned value 'len-offset' here.
int diff = len - offset;
^
src/util-memcmp.h:318:33: note: If condition 'len-offset<16' is true, the function will return/exit
if (likely(len - offset < 16)) {
^
src/util-memcmp.h:344:18: note: Testing identical condition 'len-offset<16'
if (diff < 16) {
^
src/util-memcmp.h:171:18: warning: Identical condition 'len-offset<16', second condition is always false [identicalConditionAfterEarlyExit]
if (diff < 16) {
^
src/util-memcmp.h:170:24: note: 'diff' is assigned value 'len-offset' here.
int diff = len - offset;
^
src/util-memcmp.h:159:33: note: If condition 'len-offset<16' is true, the function will return/exit
if (likely(len - offset < 16)) {
^
src/util-memcmp.h:171:18: note: Testing identical condition 'len-offset<16'
if (diff < 16) {
^
src/util-memcmp.h:233:18: warning: Identical condition 'len-offset<16', second condition is always false [identicalConditionAfterEarlyExit]
if (diff < 16) {
^
src/util-memcmp.h:232:24: note: 'diff' is assigned value 'len-offset' here.
int diff = len - offset;
^
src/util-memcmp.h:208:33: note: If condition 'len-offset<16' is true, the function will return/exit
if (likely(len - offset < 16)) {
^
src/util-memcmp.h:233:18: note: Testing identical condition 'len-offset<16'
if (diff < 16) {
^
Set correct direction for PORT mode, where the server connects
to the client.
The direction is not also strictly enforced. No data in the wrong
direction will be accepted to setup the file or to be added to the
file after setup.
This also fixes files getting closed twice.
Adds some general cleanups.
Bug: #3542.
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
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
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.
Victor Julien
Shivani Bhardwaj
Philippe Antoine
Jason Ish
Juliana Fajardini
Victor Julien
Scott Jordan
Eric Leblond
Arne Welzel
Sascha Steinbiss