Introduce the tcp.session: keyword that accepts a comma-separated subset
of {setup, established, closing} and matches packets whose TCP session
state falls within the named phases.
This lets a rule writer cover the full TCP lifecycle in one rule instead
of separate flow:not_established + flow:established rules.
Changes:
- New detect-tcp-session.{c,h} with parser, match function, unit tests
- Register DETECT_TCP_SESSION in detect-engine-register.{c,h}
- Add to Makefile.am source list
- Extend engine-analyzer text output for tcp.session:
- Extend firewall.json keyword_info with tcp_session phase values
- Update flow-keywords.rst documentation
- Require TCP protocol and SIG_FLAG_REQUIRE_PACKET at setup
Ticket: 7704
(cherry picked from commit 398687d5d4)
Extend the app-layer-protocol keyword to accept a pipe-separated list of
protocol values, so a single rule can match any of several protocols:
app-layer-protocol:[!]<proto1>|<proto2>[|...][,<qualifier>]...;
A non-negated list matches when the flow's protocol equals any listed value
(OR); a negated list matches when it equals none of them (NOR). The
single-value form and the trailing mode qualifier are unchanged.
Matching keeps the historical AppProtoEquals() equivalences by default
(dns/doh2, http/http1/http2, dcerpc/smb, ...). An `exact` qualifier selects
strict identity matching with no equivalences and no http umbrella; it
combines with a direction mode in any order. Because a flow is never the
generic ALPROTO_HTTP, `http,exact` is rejected at load.
Values are expanded once at rule load into an effective match-set bitmask, so
the per-packet match is a single bitmask test. Single-value rules remain
prefilterable; multi-value rules are excluded from prefiltering and an
explicit prefilter on them is rejected. Conflicting keyword combinations
(duplicate or overlapping negations, mixed positive/negated) are rejected at
load. Engine-analysis reports the effective match set.
Ticket: 7705
(cherry picked from commit 43bc2db41e)
The fallback memrchr() implementation initialized the search pointer to
the one-past-end position and dereferenced it before decrementing, which
is undefined behavior in C.
Update the implementation to decrement the pointer before dereferencing
while preserving memrchr() semantics. Compare bytes using unsigned char
semantics to ensure values with the high bit set are handled correctly.
Refactor the fallback into a local SCMemrchrFallback() helper so it can
be exercised by unit tests on platforms that provide a native memrchr()
implementation.
Expand the unit test to cover first, middle, last, duplicate, single-byte,
zero-length, absent-element, and high-byte values.
Ticket: 9010
Signed-off-by: Urval Kheni <kheniurval777@gmail.com>
(cherry picked from commit b475cc25b6)
Substitute an empty string for a NULL key or reference in
AlertJsonReference() before the write, so the output no longer depends
on what a libc does with a null pointer and "%s", which the standard
leaves undefined. glibc prints "(null)"; nothing requires that.
Assert both are set alongside the substitution. DetectReferenceParse()
is the only producer of DetectReference and sets each string with its
length, so the assert cannot fire today -- it names the invariant for a
second producer that does not hold it, while the substitution covers a
release build, where DEBUG_VALIDATE_BUG_ON compiles out.
A release build that reaches the substitution logs whichever half it
has rather than crashing, and nothing records that it happened.
Issue: 8849
(cherry picked from commit 9cffa8ab49)
Assign ref->key for a reference key that is not in reference.config, so
the fallback that registers a synthetic entry produces the same key and
key_len as an ordinary lookup. The assignment sat in the branch taken
when SCRConfGetReference() found the key; the fallback registered the
entry, looked it up again, and fell through with ref->key still at the
NULL SCCalloc() left.
AlertJsonReference() passed that key to snprintf() with "%s%s" and
sized the destination from key_len. With key NULL and key_len 0, a rule
carrying reference:unknownkey,001-2010 logged "(null)001-" on glibc,
and handed a null pointer to a "%s" conversion, which the standard
leaves undefined.
Invert the lookup test so the add-and-retry runs when the lookup fails
and both paths reach one assignment. The comment on that assignment
named REFERENCE_SYSTEM_NAME_MAX; the string copied is a URL, bound by
REFERENCE_CONTENT_NAME_MAX.
DetectReferenceParseTest03() loaded a rule with an unknown key and
checked only that the signature parsed, which it did throughout. Check
the resolved key, its length, and the reference value.
Issue: 8849
(cherry picked from commit 6216f9262c)
A server can reject a BDAT command before the client sends the
advertised chunk. Suricata would keep waiting for those bytes and treat
the rest of the SMTP session as chunk data.
Keeps subsequent SMTP commands and message data in sync with the
server.
Ticket: 8995
(cherry picked from commit 51ac40009f)
Suricata never removed the unix command socket file after shutdown.
The socket path (sockettarget) is now preserved in the struct
UnixCommand_, because it is needed on shutdown to close the socket.
The cleanup is done in the shutdown path of UnixManager(), which is
reached uniformly whether shutdown is triggered by a fatal error or
a signal (e.g. SIGTERM) via THV_KILL. This also applies for a clean
shutdown via 'suricatasc -c shutdown'.
Ticket: 8799
(cherry picked from commit 011f52379c)
And log it with from_utf8_lossy
Since we may now cut a too big failure_reason breaking a valid
utf-8 string
Ticket: 8731
(cherry picked from commit c608165c04)
Ticket: 8731
Adds a configurable limit to string lengths to avoid to retain
too much memory for too long, and avoid producing log events
that are too big
(cherry picked from commit f9515dc71d)
Previously, hook sequences in both rules and default policies
allowed `pass` to be combined with incompatible hooks (e.g.
reject or drop).
This commit clamps pass to accept only.
Ticket: 8955
(cherry picked from commit d6521adf4e)
A secondary action given without an explicit scope inherits the scope of
the primary action, but was never validated against the scopes it
supports itself. `pass` only supports packet and flow scope but
the inheritance was never verified.
Ticket: 8955
(cherry picked from commit 4bafe61ecf)
Some firewall rule analysis output had hardcoded paths.
This commit is querying respective tables and uses the actual
values.
Ticket: 8952
(cherry picked from commit 25ece1ed38)
When multithreaded output was enabled, all threads freed
shallow-copy of sensor and prefix names.
This commit adds a guard so that only the parent frees the
allocated buffers.
Ticket: 9018
(cherry picked from commit 7ca9471145)
Add support for ERF record types META and PAD, plus additional ETH types.
Add support for ERF extension headers.
These types and extension headers were already supported by erf-source-dag.
Made source-erf-file and source-erf-dag more consistent.
Ticket: 8962
Fixed merge conflicts on backport
(cherry picked from commit 3d42f7cf63)
When reading an ERF file do not trust the ERF header values.
Do not use wlen to set packet length.
Ticket: 8836
Read rlen bytes into MAX_PAYLOAD_SIZE buffer.
Use PacketCopyData() to safely update p.
Ticket: 8865
(cherry picked from commit c6dd80f787)
SSLv2Decode() consumed six fixed CLIENT_HELLO body bytes (version +
cipher_spec_length + session_id_length) guarded only by input_len >= 6,
which checks the buffer remainder but not the record itself. When an
attacker sends a record whose declared record_length is < 7 (e.g. 1),
bytes_processed advances past record_lengths_length + record_length,
leading an integer underflow.
Address by adding a record_length >= 7 check to the CLIENT_HELLO case.
Also add another check: bail if bytes_processed > record_length +
record_lengths_length.
Ticket: #8853.
(cherry picked from commit 928ac01215)
Improper pointer handling would leave dangling pointers after allocation
failure that could get freed again (double free).
Ticket: #8857.
(cherry picked from commit 201ecd5618)
nfs4_res_secinfo_no_name() used a local var "i2" to walk through
the security-flavor array entries, but returned the pre-walk var "i"
in the Ok at the end. Every byte of the flavor array was therefore left
unconsumed in the input stream and re-interpreted by count next iteration
as a subsequent compound operation.
Addressed by returning i2 instead of i so all flavor bytes are consumed.
Ticket: #8860.
(cherry picked from commit 9b491a131e)
Rust website has changed the location of the installer.
Additionally their installer now includes automatic append to
.bashrc to source the correct path to Rust binaries.
Cbindgen as an instruction is also removed as it is not needed
when building from the downloaded and packaged Suricata.
Ticket: 8344
(cherry picked from commit d7bf52e71c)
The crash handler built its stack trace by advancing a pointer with
the snprintf return value. That value is the length the output would
have had, not what was written, so once the trace filled msg the
pointer moved past the end of the buffer. Every size argument after
that, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), was negative and
converted to a huge size_t, and snprintf went on writing into the
stack past msg. With a deep enough stack the handler faults before it
can log the crash it exists to report.
The frame name lookup also compared unw_get_proc_name() against
UNW_ENOMEM, but libunwind returns the negated code, so the test
never fired. A lookup that failed with UNW_ENOINFO fell through
to the success branch and printed the name buffer, which
libunwind had not written.
Track space used with offset and use that on each snprintf call. Advance
by MIN(cw, remaining - 1) to prevent overruns. The loop stops when
the buffer is full and closes the trace with "..." so a clipped message
is not read as a whole stack.
Failed symbol lookups now print "[unknown]:". -UNW_ENOMEM is not one of
them, since libunwind returns a truncated but usable name with it.
name starts as "?" so nothing is printed from it either way.
Ticket: 8846
(cherry picked from commit cd7f7751d5)
Record beside the existing note about division by zero that << and >>
give 0 when rvalue is 64 or more, that a rule giving such an rvalue as
a number still loads with a warning, and that --strict-rule-keywords
makes it fail to load instead. rvalue can name a byte_extract or
byte_math variable rather than a number, and then the count is not
known until the rule runs, so the rule text does not say whether it
will reach 64.
The note on main stops at the result, because 89d09c457c rejects such
a rule there instead of warning about it.
Issue: 8845
Issue: 8902
(cherry picked from commit 4a647784c7)
Warn from DetectByteMathSetup() when byte_math pairs << or >> with a
literal rvalue of 64 or more, naming the signature by sid. Such a
shift gives 0 for every packet: DetectByteMathDoMatch() zeroes the
result once the count reaches 64, the width of the uint64_t being
shifted. rvalue was bounded only to the u32 range, so the rule loaded
and ran a shift whose result was 0 whatever the packet held.
The rule still loads. main rejects it in 89d09c457c, the commit this
one is derived from, but a released branch should not stop loading
rules that loaded before. Passing byte_math to --strict-rule-keywords
turns the warning into a load failure, the same escalation classtype
and reference apply to values they cannot check.
A variable rvalue draws no warning. It resolves to a preceding
byte_extract or byte_math result, whose value is known only once the
rule runs, so the guard in DetectByteMathDoMatch() stays the only
check on that path.
Issue: 8845
Issue: 8902
(cherry picked from commit 89d09c457c)
Zero the result when a byte_math right shift count reaches 64, the
width of the uint64_t being shifted, so the operation no longer
depends on behavior C11 6.5.7p3 leaves undefined. The left shift case
has done this since 473ca6dcf4; the right shift case was left
unguarded.
DetectByteMathDoMatch() shifted by whatever count it was handed. On
x86_64 the hardware masks the count to its low six bits, so a count of
64 became a shift of 0 and returned the extracted value unchanged
instead of 0. That value is stored in det_ctx->byte_values[] and feeds
any byte_test, isdataat, or content offset later in the signature, so
the signature's verdict follows from an arithmetic result the standard
does not define.
The count reaches the shift from the wire. When byte_math names a
variable for rvalue, DetectEngineContentInspectionInternal() reads it
out of det_ctx->byte_values[] at
detect-engine-content-inspection.c:614, where a preceding byte_extract
stored bytes taken from the payload, so one payload byte of 0x40 sets
the count to 64.
Issue: 8845
Issue: 8902
(cherry picked from commit e5d035fd16)
Setting a cpu-affinity path with --set, like
--set threading.cpu-affinity.worker-cpu-set.threads=28
creates an intermediate node under cpu-affinity whose val is NULL.
With a legacy list format cpu-affinity, AffinitySetupLoadFromConfig()
red the set name from the node val and passed it to
GetAffinitySetName(), which then dereferenced the NULL pointer.
Address it by simply checking for the NULL.
Ticket: #6735.
(cherry picked from commit 54504ed13c)
The fn to flush the ring is supposed to flush the entire ring buffer but
it only resets the buffer up to the "length" bytes. Fix this calculation
to correctly reflect the number of bytes that need to be reset.
Ticket: 8826
(cherry picked from commit 389700eca2)
Ticket: 8678
With an inspect window of 0xAAAAAAAB the `window * 3` guard wrapped to 1
in uint32_t arithmetic, so it passed for a 64 byte file and
content_inspected was set to `file->size - window`, an underflow.
(cherry picked from commit 8e551f2860)
Ticket: 8678
In FilePruneFile() `window * 3` was computed in uint32_t arithmetic.
The guard exists to ensure `file->size > window`, so on wrap around
`file->size - window` underflows and content_inspected is set to a
bogus value, breaking file inspection.
Make window uint64_t so the multiplication cannot wrap.
(cherry picked from commit f3910d557d)
Validate the resolved scope against the class of hook it is being applied
to and fail at startup if it does not fit.
Ticket: 8770
(cherry picked from commit 092ae272e2)
Every hook has a built-in default policy, but expressing anything other
than the built-in meant naming each hook explicitly.
This commit adds a `default-policy` setting that covers all hooks below it.
For any hook the most specific setting present wins.
Ticket: 8770
(cherry picked from commit 0aaa80d4f3)
Mapping an app-layer progress state to its generic request-/response-
hook alias was hardcoded in multiple places.
This commit adds a wrapper to unify it to a single function.
It returns the config-form (hyphenated) name, or NULL for an
intermediate state.
Ticket: 8770
(cherry picked from commit 1aa0259876)
Use for IPv6 in the main hash.
Update IPv4 thread cache to also use a seed. For this the thread
initialization is done at thread init instead on the first packet. The
thread ctx is no longer stored in a thread_local, but instead uses the
thread storage API to store in the ThreadVars. This way it will stay
active during rule reloads as well.
Ticket: #8617.
(cherry picked from commit c27868a8c5)
Yash Datre
Urval Kheni
Jeff Lucovsky
Jason Ish
Andreas Dolp
Philippe Antoine
Lukas Sismis
Stephen Donnelly
Victor Julien
Shivani Bhardwaj
Denis Balashov