Bug 5458 states that the reject action is no longer working. While SV
tests that use the reject action still pass, it indeed seems that a
regression has happened with commit aa93984, because while the
function that applies rule actions to the flow (RuleActionToFlow) does
check for the reject action, the newly added function PacketApply
SignatureActions only checks for ACTION_DROP or ACTION_PASS when
deciding to call RuleActionToFlow.
A Packet may be dropped due to several different reasons. This change
adds action as a parameter, so we can update the packet action when we
drop it, instead of setting it to drop.
source/pcap: fix infinite loop if interface goes down
When in live-pcap mode, if the sniffed interface went down and up again,
Suri would enter an infinite and keep running, while not registering new
events. This fixes that behavior by allowing Suri to retry to open the
pcap in case of a retry on an already activated capture
('PCAP_ERROR_ACTIVATED').
detect/engine: init alert queue counters on reload
alert_queue_overflow and alerts_suppressed were not being
reinitialized when there was a reload of Suricata rules, leading to
non-valid stats counters if that happened.
> The `count` combinator preallocates a number of bytes. Since the value
> is untrusted, this can result in an Out Of Memory allocation.
> Use a maximum value, large enough to cover all current implementations.
Jason Ish [Wed, 20 Jul 2022 19:07:49 +0000 (13:07 -0600)]
readthedocs: add configuration file
Readthedocs is attempting to use our /requirements.txt file as a Python
requirements file, which it is not. Add a Readthedocs configuration file
to tell it to not use any requirements file.
When switching from SMTP to TLS, and getting HTTP1 instead of
expected TLS, and HTTP1 requesting upgrade to HTTP2, we do not
overwrite the alproto_orig value so as not to have type confusion
in AppLayerParserStateProtoCleanup
Philippe Antoine [Tue, 31 May 2022 13:49:38 +0000 (15:49 +0200)]
events: reset logged event
Otherwise, if we log a first app_layer_event, then reset
with AppLayerDecoderEventsResetEvents for a new packet,
then get another event, it does not get logged
Victor Julien [Tue, 28 Jun 2022 18:20:37 +0000 (20:20 +0200)]
fuzz/mime: fix call conditions and args
The SMTP parser should not supply lines w/o EOL chars to the mime
parser unless its in the BODY parsing stage. Mimic this in the fuzz
target by testing the state for inputs that have no EOL.
Additionally, make sure the delim cnt reflects the missing EOL.
Victor Julien [Fri, 10 Sep 2021 11:43:26 +0000 (13:43 +0200)]
tests: fix drop test; cleanup
SigTestDropFlow04 was incorrectly expecting an alert in the packet
following a "drop" packet. The first drop is applied to the flow, so
it should lead to the 2nd packet being dropped before inspection is
run.
Adds a framework for setting exception policies. These would be called
when the engine reaches some kind of exception condition, like hitting
a memcap or some traffic processing error.
The policy gives control over what should happen next: drop the packet,
drop the packet and flow, bypass, etc.
Implements the policy for:
stream: If stream session or reassembly memcaps are hit call the
memcap policy on the packet and flow.
flow: Apply policy when memcap is reached and no flow could be
freed up.
defrag: Apply policy when no tracker could be picked up.
app-layer: Apply ppolicy if a parser reaches an error state.
All options default to 'ignore', which means the default behavior
is unchanged.
Adds commandline options: add simulation options for exceptions. These
are only exposed if compiled with `--enable-debug`.
Lukas Sismis [Fri, 20 May 2022 19:33:38 +0000 (21:33 +0200)]
bypass: fix memory leak - reassign of FlowBypassInfo
In some situations bypass callback is called on already bypassed
flow. This allocates FlowBypassInfo structure for the flow but
does not check if the flow already has one.
Victor Julien [Tue, 7 Jun 2022 20:57:39 +0000 (22:57 +0200)]
detect/threshold: fix offline time handling issue
Due to the TIMEVAL_DIFF_SEC calculating the delta into an unsigned
integer, it would underflow to a high positive value leading to
and incorrect result if the packet timestamp was below the timestamp
for the threshold entry. In normal conditions this shouldn't happen,
but in offline mode each thread has its own concept of time which
might differ significantly based on the pcap. In this case the
overflow would be very common.
Changing it to a signed value calculation triggered fuzz undefined
behavior if the packet timeval was very high, so this patch takes a
new approach where it no longer calculates a diff but sets up the
"seconds" value we compare against as a timeval itself, and uses
that to compare.
Fixes: 9fafc1031c0c ("time: Add TIMEVAL_EARLIER and TIMEVAL_DIFF_SEC macros.") Fixes: 82dc61f4c3e3 ("detect/threshold: Refactor threshold calculation to handle by_rule and by_both.")
Uses add `timeradd` specific version where available.
Victor Julien [Wed, 8 Jun 2022 11:11:55 +0000 (13:11 +0200)]
stream/midstream: fix double flow reverse case
In the case of midstream SYN/ACK pickup, we reverse the flow based on
the SYN/ACK. If we then later get traffic that appears to be in the
reverse direction based on the app-layer, we would reverse it again.
This isn't correct. When we have the SYN/ACK we know the flow's real
direction.
Michael Tremer [Fri, 11 Mar 2022 11:08:10 +0000 (11:08 +0000)]
stream: tcp: Handle retransmitted SYN with TSval
For connections that use TCP timestamps for which the first SYN packet
does not reach the server, any replies to retransmitted SYNs will be
tropped.
This is happening in StateSynSentValidateTimestamp, where the timestamp
value in a SYN-ACK packet must match the one from the SYN packet.
However, since the server never received the first SYN packet, it will
respond with an updated timestamp from any of the following SYN packets.
The timestamp value inside suricata is not being updated at any time
which should happen. This patch fixes that problem.
Victor Julien [Thu, 9 Jun 2022 20:25:44 +0000 (22:25 +0200)]
detect/content: fix FNs due to bad depth calc
When trying to propegate the depth/offset, within/distance chains
a logic error would set too a restrictive depth on a pattern that
followed more than one "unchained" patterns.
Victor Julien [Thu, 9 Jun 2022 20:25:08 +0000 (22:25 +0200)]
detect/content: simplify int bounds checking
Use a macro to validate the ranges for overflows. This removes
the clutter of all the checks and warnings, and also no longer
puts the state machine in an undefined state when hitting such
a condition.
Jason Ish [Wed, 11 May 2022 17:23:24 +0000 (11:23 -0600)]
detect: introduce "like" ip-only signature type
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.
base64: make decoder handle decoded data space constraints
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.
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.
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.
Victor Julien [Fri, 22 Apr 2022 16:27:15 +0000 (18:27 +0200)]
stream: improve last_ack validation check
If a packet after the initialization would come with ACK flag set
but a ACK value of 0, the last_ack tracking could get confused. Fix
this by not checking for 0 but instead checking if the ACK flag
has been seen.
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) {
^
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 [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 [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 [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: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.