*** CID 1358023: Null pointer dereferences (REVERSE_INULL)
/src/util-mpm-hs.c: 860 in SCHSDestroyThreadCtx()
854 if (thr_ctx->scratch != NULL) {
855 hs_free_scratch(thr_ctx->scratch);
856 mpm_thread_ctx->memory_cnt--;
857 mpm_thread_ctx->memory_size -= thr_ctx->scratch_size;
858 }
859
>>> CID 1358023: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "mpm_thread_ctx->ctx" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
860 if (mpm_thread_ctx->ctx != NULL) {
861 SCFree(mpm_thread_ctx->ctx);
862 mpm_thread_ctx->ctx = NULL;
863 mpm_thread_ctx->memory_cnt--;
864 mpm_thread_ctx->memory_size -= sizeof(SCHSThreadCtx);
865 }
Direct leak of 80 byte(s) in 5 object(s) allocated from:
#0 0x4c673b in __interceptor_malloc (/home/victor/dev/suricata/src/suricata+0x4c673b)
#1 0xb7a425 in DetectEngineSignatureIsDuplicate /home/victor/dev/suricata/src/detect-parse.c:1715:10
#2 0xb79390 in DetectEngineAppendSig /home/victor/dev/suricata/src/detect-parse.c:1836:19
#3 0x86fe56 in DetectLoadSigFile /home/victor/dev/suricata/src/detect.c:357:15
#4 0x815fee in ProcessSigFiles /home/victor/dev/suricata/src/detect.c:419:13
#5 0x8139a8 in SigLoadSignatures /home/victor/dev/suricata/src/detect.c:499:15
#6 0xfe435d in LoadSignatures /home/victor/dev/suricata/src/suricata.c:1979:9
#7 0xfcd87e in main /home/victor/dev/suricata/src/suricata.c:2345:17
#8 0x7fb66bf7cec4 in __libc_start_main /build/eglibc-3GlaMS/eglibc-2.19/csu/libc-start.c:287
By default, hashlittle() will read off the end of the key, up to the
next four-byte boundary, although the data beyond the end of the key
doesn't affect the hash. This read causes uninitialized read warnings
from Valgrind and Address Sanitizer.
Here we add hashlittle_safe(), which avoids reading off the end of the
buffer (using the code inside the VALGRIND-guarded block in the original
hashlittle() implementation).
Capture methods that are non blocking will still not generate packets
that go through the system if there is no traffic. Some maintenance
tasks, like rule reloads rely on packets to complete.
This patch introduces a new thread flag, THV_CAPTURE_INJECT_PKT, that
instructs the capture thread to create a fake packet.
The capture implementations can call the TmThreadsCaptureInjectPacket
utility function either with the packet they already got from the pool
or without a packet. In this case the util func will get it's own
packet.
Implementations for pcap, AF_PACKET and PF_RING.
Split wait loop into three steps:
- first insert pseudo packets
- 2nd nudge all capture threads to break out of their loop
- third, wait for the detection thread contexts to be used
Interupt capture more than once if needed
Move packet injection into util func
Simplify handling of USR2 signal. The SCLogInfo usage could lead to
dead locks as the SCLog API can do many complicated things including
memory allocations, syslog calls, libjansson message construction.
If an existing malloc call was interupted, it could lead to the
following dead lock:
0 __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:97
1 0x0000003140c7d2df in _L_lock_10176 () from /lib64/libc.so.6
2 0x0000003140c7ab83 in __libc_malloc (bytes=211543457408) at malloc.c:3655
3 0x0000003140c80ec2 in __strdup (s=0x259ca40 "[%i] %t - (%f:%l) <%d> (%n) -- ") at strdup.c:43
4 0x000000000059dd4a in SCLogMessageGetBuffer (tval=0x7fff52b47360, color=1, type=SC_LOG_OP_TYPE_REGULAR, buffer=0x7fff52b47370 "", buffer_size=2048,
log_format=0x259ca40 "[%i] %t - (%f:%l) <%d> (%n) -- ", log_level=SC_LOG_INFO, file=0x63dd00 "suricata.c", line=287, function=0x640f50 "SignalHandlerSigusr2StartingUp", error_code=SC_OK,
message=0x7fff52b47bb0 "Live rule reload only possible after engine completely started.") at util-debug.c:307
5 0x000000000059e940 in SCLogMessage (log_level=SC_LOG_INFO, file=0x63dd00 "suricata.c", line=287, function=0x640f50 "SignalHandlerSigusr2StartingUp", error_code=SC_OK,
message=0x7fff52b47bb0 "Live rule reload only possible after engine completely started.") at util-debug.c:549
6 0x000000000057e374 in SignalHandlerSigusr2StartingUp (sig=12) at suricata.c:287
7 <signal handler called>
8 _int_malloc (av=0x3140f8fe80, bytes=<value optimized out>) at malloc.c:4751
9 0x0000003140c7ab1c in __libc_malloc (bytes=296) at malloc.c:3657
10 0x0000000000504d55 in FlowAlloc () at flow-util.c:60
11 0x00000000004fd909 in FlowInitConfig (quiet=0 '\000') at flow.c:454
12 0x0000000000584c8e in main (argc=6, argv=0x7fff52b4a3b8) at suricata.c:2300
This patch simply sets a variable and lets the main loop act on that.
If a TCP packet could not get a flow (flow engine out of flows/memory)
and there were *only* TCP inspecting rules with the direction
explicitly set to 'to_server', a NULL pointer deref could happen.
PacketPatternSearchWithStreamCtx would fall through to the 'to_client'
case which was not initialized.
Add "ippair" autofp scheduler to split traffic based on source and
destination IP only (not ports).
- This is useful when using the "xbits" feature to track events
that occur between the same hosts but not necessarily the same
flow (such as exploit kit landings/expoits/payloads)
- The disadvantage is that traffic may be balanced very unevenly
between threads if some host pairs are much more frequently seen
than others, so it may be only practicable for sandbox or pcap
analysis
- not tested for IPv6
See https://redmine.openinfosecfoundation.org/issues/1661
This possibly fix:
ndirect leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x4c264b in malloc (/home/victor/qa/buildbot/donkey/z600fuzz/Private/src/.libs/lt-suricata+0x4c264b)
#1 0x7fb09c1e8aaa in json_array (/usr/lib/x86_64-linux-gnu/libjansson.so.4+0x6aaa)
#2 0xd67553 in JsonEmailLogJsonData /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-json-email-common.c:290:27
#3 0xd6a272 in JsonEmailLogJson /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-json-email-common.c:370:19
#4 0xd956b9 in JsonSmtpLogger /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-json-smtp.c:103:9
#5 0xdcedac in OutputTxLog /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-tx.c:165:17
#6 0xff6669 in TmThreadsSlotVarRun /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:132:17
#7 0xffecc1 in TmThreadsSlotVar /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:474:17
#8 0x7fb09bfcc181 in start_thread /build/eglibc-3GlaMS/eglibc-2.19/nptl/pthread_create.c:312
In JsonEmailLogJsonData function, an invalid state was leading to
early exit without a proper freeing of resources.
This should fix:
Indirect leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x4c264b in malloc (/home/victor/qa/buildbot/donkey/z600fuzz/Private/src/.libs/lt-suricata+0x4c264b)
#1 0x7fb09c1e886a in json_object (/usr/lib/x86_64-linux-gnu/libjansson.so.4+0x686a)
#2 0xd6a272 in JsonEmailLogJson /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-json-email-common.c:370:19
#3 0xd956b9 in JsonSmtpLogger /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-json-smtp.c:103:9
#4 0xdcedac in OutputTxLog /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/output-tx.c:165:17
#5 0xff6669 in TmThreadsSlotVarRun /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:132:17
#6 0xffecc1 in TmThreadsSlotVar /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:474:17
#7 0x7fb09bfcc181 in start_thread /build/eglibc-3GlaMS/eglibc-2.19/nptl/pthread_create.c:312
[src/util-ip.c:104]: (error) Shifting a negative value is undefined behaviour
[src/util-radix-tree.c:1160]: (error) Shifting a negative value is undefined behaviour
[src/util-radix-tree.c:1357]: (error) Shifting a negative value is undefined behaviour
[src/util-radix-tree.c:1380]: (error) Shifting a negative value is undefined behaviour
[src/util-radix-tree.c:1438]: (error) Shifting a negative value is undefined behaviour
Reset counters_global_id at ctx destruction. In the unix socket
runmode the lack of this reset would cause the id's to increase with
each pcap, leading to an ever larger stats array.
Denotes the max detection list so that rule validation can
allow post-detection lists to come after base64_data, but
disallow detection lists to come after it.
As SMTP file_data detection uses the file API, the file's inspect
tracker should be considered when pruning files.
This patch sets the FILE_USE_DETECT flag on files tracked by smtp.
It also adds logic to move inspected tracker ahead if detection
doesn't do it, like when no rules are matching or detection engine
is disabled.
When the file API is used to do content inspection (currently only
smtp does this), the detection should be considered while pruning
the file chunks.
This patch introduces a new flag for the file API: FILE_USE_DETECT
When it is used, 'FilePrune' will not remove chunks that are (partly)
beyond the File::content_inspected tracker.
When using this flag, it's important to realize that when the detect
engine is disabled or rules are not matching, content_inspected
might not get updated.
Cppcheck 1.72 gives a warning on the following code pattern:
char blah[32] = "";
snprintf(blah, sizeof(blah), "something");
The warning is:
(error) Buffer is accessed out of bounds.
While this appears to be a FP, in most cases the initialization to ""
was unnecessary as the snprintf statement immediately follows the
variable declaration.
Mats Klepsland
Victor Julien
browner87
Jason Ish
Justin Viiret
cdwakelin
Eric Leblond
JonZeolla
gureedo