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
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
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
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
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.
SCACTileSearchLarge uses 32-bit next-state cells holding a 24-bit state
index. When there are more than 65536 states, CheckMatch was called with
(uint16_t)state, silently discarding the bits beyond 16.
Address this by widening CheckMatch's state parameter.
Ticket: #8841.
Prevent out-of-bounds read of protocol field when PPPoE session header is
exactly PPPOE_SESSION_HEADER_MIN_LEN bytes. Inspect first protocol octet
only to decide single vs double octet form, and read second octet only after
len >= 8 verification.
Add unit tests for the 7-byte boundary case for both the double-octet
(rejected as too small) and single-octet (decoded as PPP_IP) protocols.
Ticket: #8856.
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: 8861
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
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
Record that << and >> yield 0 when rvalue is 64 or more, beside the
existing note about division by zero. A shift count can come from a
byte_extract variable, so the rule text alone does not say which
counts land in that case.
Issue: 8845
Fail rule load when byte_math pairs << or >> with a literal rvalue of
64 or more. rvalue was bounded only to u32::MAX, so a rule shifting by
100 loaded and then produced 0 for every packet it inspected, spending
detection work on a comparison whose outcome was settled before the
first packet arrived.
A variable rvalue still loads. Its value comes from a byte_extract on
the packet and is not known until the rule runs, so the guard in
DetectByteMathDoMatch() stays the only check covering that path.
DetectByteMathParse() reports every SCByteMathParse() failure as
"invalid bytemath values", so the rejected rule is named by the
"error parsing signature" line that follows rather than by the reason
the parser gave.
Issue: 8845
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
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
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.
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.
DoParseAppSubStatePolicy() collapses into DoParseAppPolicy() as a sub state
hook only differs by an extra path segment.
Ticket: 8712
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: 8712
Previously, a rule that had firewall-only syntax or keywords would fail
the script check.
Since we can't guarantee that a firewall rule will look different than a
detection one, run rule examples against both scenarios before failing
them.
If the point is to check the docs for added changes, it makes sense to
that used binary is the one that comes with the doc changes. Thus, try
using the local binary first, before falling back to usr/bin installed
Suricata.
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
IPOnly Engine does not support parsing and recording IPv6 ranges. So,
passing a range of ipv6 there just results in non-fatal errors in
parsing the IP addresses which means they fail to be added to the radix
tree struct.
Re-classify the ipv6 ranges as like_ip_only so they fall out of the
IPOnly engine and are processed per packet.
Ticket: 8847
Add accessor functions to allow NDPI and our plugin examples to access
flow fields in an opaque manner.
Provides accessors for:
- is ipv4
- is ipv6
- ip protocol
- to server packet count
- to destination packet count
Ticket: #8632
Remove the check for Flow.storage being NULL. With flex arrays, as long as the
flow is allocated, .storage will never be NULL. We do make sure the flow
is not NULL, and the storage functions are safe to the storage being
NULL.
Prevents NDPI from accessing this flow field to be less sensitive to ABI
changes with respect to data structure sizes.
Ticket: #8632
dependabot[bot]
Jeff Lucovsky
Philippe Antoine
Lukas Sismis
Victor Julien
Stephen Donnelly
Shivani Bhardwaj
Jason Ish
Juliana Fajardini
Yash Datre