Added a page that explains how rules are prioritized by Suri, as well
as what main different types of inspection happen and what elements are
involved when ordering rules.
Victor Julien [Wed, 10 Sep 2025 14:06:11 +0000 (16:06 +0200)]
unix-socket: address scan-build warning
CC unix-manager.o
unix-manager.c:258:13: warning: Use of memory after it is freed [unix.Malloc]
258 | if (item->fd == fd) {
| ^~~~~~~~
1 warning generated.
Shivani Bhardwaj [Fri, 20 Jun 2025 09:57:27 +0000 (15:27 +0530)]
doc: add doc on internals of inspection of raw data
Explain briefly the internals of inspection of raw data in the following order:
- Stream Engine
- Stream reassembly
- Role of Detection Engine and Applayer Parsers
- High level communication between Stream and Detection Engine
- Relevant suricata.yaml settings
Add information about:
- available tables, default policies and rule ordering
- Packet layer and applayer tables and hooks
- engine analysis output
- commandline options available
- how to load firewall rules
Also, reorganize sections and content to assist the definitions.
Victor Julien [Wed, 3 Sep 2025 16:38:11 +0000 (18:38 +0200)]
stream: workaround scan-build warnings
stream-tcp.c:1938:16: warning: Access to field 'next' results in a dereference of a null pointer (loaded from variable 'tail') [core.NullDereference]
1938 | tail->next = old_head;
| ~~~~ ^
1 warning generated.
stream-tcp.c:1982:5: warning: Potential leak of memory pointed to by 'q' [unix.Malloc]
1982 | ssn->queue_len++;
| ^~~
1 warning generated.
Victor Julien [Mon, 1 Sep 2025 12:51:56 +0000 (14:51 +0200)]
stream: add more liberal timestamp behavior in 3WHS
RFC 7323 forbids a server to respond with a timestamp option in the
SYN/ACK when the SYN didn't have a timestamp option:
A TCP MAY send the TSopt in an initial <SYN> segment (i.e., segment
containing a SYN bit and no ACK bit), and MAY send a TSopt in
<SYN,ACK> only if it received a TSopt in the initial <SYN> segment
for the connection.
Once TSopt has been successfully negotiated, that is both <SYN> and
<SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
segment for the duration of the connection, and SHOULD be sent in an
<RST> segment (see Section 5.2 for details).
However, in the real world this pattern happens on benign traffic. This
would lead to missing logs and detection, and in IPS mode such sessions
would be blocked.
This patch allows this pattern when the `stream.liberal-timestamps` is
enabled (enabled by default).
Victor Julien [Wed, 20 Aug 2025 10:43:27 +0000 (12:43 +0200)]
stream: improve SYN and SYN/ACK retransmission handling
Take SEQ and ACK into account for more scenarios.
SYN on SYN_SENT
In this case the SYN packets with different SEQ and other properties are
queued up. Each packet updates the ssn to reflect the last packet to
come in. The old ssn data is added to a TcpStateQueue entry in
TcpSession::queue. If the max queue length is exceeded, the oldest entry
is evicted. The queue is actually a single linked list, where the list
head reflects the oldest entry.
SYN/ACK on SYN_SENT
In this case the first check is if the SYN/ACK matches the session. If
it doesn't, the queue is checked to see if there SYN's stored. If one is
found that matches, it is used and the session is updated to reflect
that.
SYN/ACK on SYN_RECV
SYN/ACK resent on the SYN_RECV state. In this case the ssn is updated
from the current packet. The old settings are stored in a TcpStateQueue
entry in the TcpSession::queue.
ACK on SYN_RECV
Checks any stored SYN/ACKs before checking the session. If a queued
SYN/ACK was sound, the session is updated to match it.
Thomas Winter [Wed, 9 Apr 2025 03:17:08 +0000 (15:17 +1200)]
decode/pppoe: Don't mark expected PPP protos as unsupported
After upgrading from 7.0.6 to 7.0.8, regular ppp packets are getting
dropped when ppp rules in decoder-events.rules were set as drop.
This was caused by commit a8f35806 ("detect: fix decoder only events").
Previously these rules would not be alerted or dropped.
It turns out several PPP protocols in a switch statement were falling
into the PPP_UNSUP_PROTO case. This has always been the case, I assume
the intention was that they don't get further inspected for size and
other decode errors hence unsupported.
But really some of the protocols are fundamentally required for a PPP
connection to take place.
Change some types that we know should be allowed to pass this.
Shivani Bhardwaj [Thu, 28 Aug 2025 02:36:44 +0000 (08:06 +0530)]
stream: remove incorrect defensive check
As a part of the commit d096b98 a defensive check was added stating that
the stream must have EOF flag set if it is in TCP_CLOSING state or
above. However, this led to a false positive reported by oss-fuzz whose
analysis showed that this does not hold true for TCP_CLOSING state. It
does hold true only for TCP_CLOSED or if packet has PKT_PSEUDO_STREAM_END
set.
TCP_CLOSING state correspond to an established flow hence the correct
course of action is to remove the assertion.
Bug 7636
Co-authored-by: Philippe Antoine <pantoine@oisf.net>
Charlie Vigue [Tue, 22 Jul 2025 08:55:45 +0000 (08:55 +0000)]
util: Fix a hash table collision bug
In util-hash.c there was some behavior that is unexpected and likely
incorrect. To see this behavior, create a hash table 32 entries wide
and use the default hash function. Then add a short string “abc”,
observe the string is stored properly. Now remove a string “iln”, and
observe string “abc” is no longer in the table.
This is because the hash function is not properly handling collisions in
some edge cases.
Includes new unit test:
- UT verifies that the hash function generates a collision for
the selected test data. This must be true for the bug to be present.
Then UT demonstrates the bug by adding two items to the hash table
that collide, and then removing one of them 2x. The bug is that the
other value is removed as well.
Jeff Lucovsky [Sun, 17 Aug 2025 14:23:44 +0000 (10:23 -0400)]
detect/from_base64: Support keyword w/no opts
Issue: 7853
Support the use of `from_base64` with no optional values. In this case,
the default values for:
- mode RFC4648
- offset: 0
- bytes: buffer size
will be used.
Jason Ish [Thu, 7 Aug 2025 15:21:31 +0000 (09:21 -0600)]
rust: fix mismatched_lifetime_syntaxes warning
Fix new warning present in Rust 1.89.
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/ldap/types.rs:191:30
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
engine/analyzer: write rule failure report to correct file
The failure report was always just written to rules_fast_pattern.txt. In
case that setting is disabled or there's nothing fast-pattern related,
the report should be written to the usual rules_analysis.txt.
engine/analyzer: check if file pointer exists before writing
de_ctx->ea->fp_engine_analysis_fp is only initialized if
engine-analysis.rules-fast-pattern is enabled in the configuration. If
this config param is missing, this leads to segfault.
Remove unused rule prefilter-related stats counters that aren't in use.
94644ac9604c (detect: move non-pf rules into special prefilter engines)
removed the logic that made use of and incremented the stats counters:
- det_ctx->counter_fnonmpm_list
- det_ctx->counter_nonmpm_list
Some code was left, registering them, and mentioning them in the
json schema.
- cleanup usage and documentation around needs
- mentiond that rule hooks are used instead of "needs" keywords with
link with rule hooks (which is still in the firewall-design doc)
Jason Ish [Tue, 29 Jul 2025 14:28:21 +0000 (08:28 -0600)]
github-ci: add almalinux 10 build
Based on the current AlmaLinux 9 build, with plugin tests, etc.
Remove cppclean as its not installed and was previously disabled with
commit 2d308c000d58dbf5323599fc7f1694e14f1f375b.
hyperscan: prevent LTO opmitizing out hash calculation
Since cached_hash was updated through reference (hash), it seems
LTO did not notice this and optimized the whole code block, returning
zero.
This in turn caused all caches to have the same name and to overwrite.
On subsequent runs, only the last cache was loaded for all SGHs
causing wrong MPM assignment.
Jason Ish [Tue, 22 Jul 2025 14:29:08 +0000 (08:29 -0600)]
github-ci: add flto build
Ubuntu and Fedora packing system build with -flto=auto by default, so
update one test to use -flto=auto. Also build with -O2 as that
combination can cause issues such as
https://redmine.openinfosecfoundation.org/issues/7824.