Victor Julien [Wed, 27 Apr 2022 13:35:46 +0000 (15:35 +0200)]
detect/state: address cppcheck warnings
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)
^
Victor Julien [Fri, 6 May 2022 15:46:40 +0000 (17:46 +0200)]
memcmp: work around GCC 12+ 'blend' issues
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.
Victor Julien [Thu, 5 May 2022 05:16:53 +0000 (07:16 +0200)]
memcmp: remove unreachable code from memcmp simd
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) {
^
On some platforms (riscv64, s390x) this value is "undefined" as returned
from getconf. We also need to handle this to avoid using the string
"undefined" blindly in further #defines.
Jason Ish [Thu, 21 Apr 2022 19:41:15 +0000 (13:41 -0600)]
scripts/bundle: use git instead of tar.gz
To better fit with our current CI processes, use git to clone the
suricata-update and libhtp dependencies. The requirements.txt file has
been modified to take a repo URL and a `-b` command line option for tag
or branch.
For the master branch we will use the libhtp 0.5.x branch and the
suricata-update master branch.
Also allows for repo and branch names to be overrided with environment
variables:
- SU_REPO
- SU_BRANCH
- LIBHTP_REPO
- LIBHTP_BRANCH
Jason Ish [Fri, 22 Apr 2022 18:04:37 +0000 (12:04 -0600)]
ftp: truncate first segment if over max length
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.
Jason Ish [Tue, 8 Feb 2022 19:42:37 +0000 (13:42 -0600)]
dns: add pdu frame
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.
Jason Ish [Tue, 26 Apr 2022 20:59:18 +0000 (14:59 -0600)]
frames(rust): don't call into C if running Rust unit tests
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.
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.
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).
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.
Jason Ish [Tue, 26 Apr 2022 19:21:40 +0000 (13:21 -0600)]
app-layer: more generic state trait
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.
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.
Victor Julien [Wed, 27 Apr 2022 09:39:27 +0000 (11:39 +0200)]
logopenfile: fix minor format string warning
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);
^
Victor Julien [Wed, 27 Apr 2022 09:38:37 +0000 (11:38 +0200)]
ja3: fix minor format string warning
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,
^
Victor Julien [Wed, 27 Apr 2022 09:36:21 +0000 (11:36 +0200)]
log-pcap: remove redundant check
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) {
^
Victor Julien [Wed, 27 Apr 2022 09:32:22 +0000 (11:32 +0200)]
af-packet/v2: use proper type for ring
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.
Victor Julien [Tue, 26 Apr 2022 19:47:37 +0000 (21:47 +0200)]
detect/pcre: assist code analyzer around pointer logic
cppcheck:
src/detect-pcre.c:381:27: warning: Either the condition 'pcap' is redundant or there is overflow in pointer subtraction. [nullPointerArithmeticRedundantCheck]
cut_capture = MIN((pcap - regexstr), (fcap - regexstr));
^
src/detect-pcre.c:378:18: note: Assuming that condition 'pcap' is not redundant
else if (pcap && !fcap)
^
src/detect-pcre.c:381:27: note: Null pointer subtraction
cut_capture = MIN((pcap - regexstr), (fcap - regexstr));
^
Victor Julien [Tue, 26 Apr 2022 19:03:42 +0000 (21:03 +0200)]
time: fix warning in timestring creation
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.
Victor Julien [Tue, 26 Apr 2022 18:36:36 +0000 (20:36 +0200)]
detect/multi-tentancy: minor format string fixes
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);
^
Victor Julien [Tue, 26 Apr 2022 18:18:28 +0000 (20:18 +0200)]
reference: remove useless var reset
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;
^
Victor Julien [Tue, 26 Apr 2022 18:17:27 +0000 (20:17 +0200)]
runmodes: minor format string fixes
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);
^
Victor Julien [Tue, 26 Apr 2022 18:14:39 +0000 (20:14 +0200)]
mpm/ac-ks: address int handling issues
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);
^
Victor Julien [Tue, 26 Apr 2022 18:12:20 +0000 (20:12 +0200)]
classification: remove useless clear
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;
^
Victor Julien [Tue, 26 Apr 2022 18:05:51 +0000 (20:05 +0200)]
detect/analyzer: minor format string fixes
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);
^
Victor Julien [Tue, 26 Apr 2022 18:04:28 +0000 (20:04 +0200)]
detect/address: remove useless checks
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.
projects / thirdparty / suricata.git / log
