The seek into the fragment tree uses frag_offset - 1 as the RB_NFIND
key. frag_offset is uint16_t, so the subtraction promotes to int, and
for the first fragment of a datagram (offset 0) the -1 narrows back to
65535 when it is stored in the key. -fsanitize=implicit-conversion
flags this.
The - 1 has to stay. DefragRbFragCompare never returns 0, so RB_NFIND
is strictly greater rather than inclusive. With key = frag_offset it
returns the fragment after an existing node at the same offset and
leaves prev on that node, so the preceding fragment, which may overlap
from the left, is skipped. With key = frag_offset - 1 the same-offset
node is included and prev ends up on the real predecessor.
So guard only the offset 0 case:
.offset = frag_offset ? frag_offset - 1 : 0,
For frag_offset == 0 the seek starts at RB_MIN either way, so this
keeps the current behaviour and drops the wrap to 65535.
Add two unit tests. DefragOutOfOrderFirstFragmentTest covers offset 0
arriving into a populated tree. DefragSameOffsetPrecedingOverlapTest
pins down the same-offset plus left-overlap case that tells
frag_offset - 1 and frag_offset apart.
Ticket: 8232
Tickete: 8265
The traffic "USER CAPABILITY" could be interpreted as both ftp
or imap.
Add a probing parser to imap after detection of the " CAPABILITY"
pattern to try do disambiguate.
While the pgsql parser is disabled if its setting is present in the yaml
config file, it is actually enabled if truly defaulted, that is, if not
present in the settings.
Actually disable pgsql parsing and logging if not present in the
suricata.yaml config. This implies that eve output enabling isn't enough
to enable the parser, now.
Bug: #8349
EngineAnalysisFP(), CleanupFPAnalyzer(), and CleanupRuleAnalyzer() were writing
to and closing the wrong FILE* pointers. Fast pattern output was written to
rules_analysis.txt instead of rules_fast_pattern.txt, leaving the latter
effectively empty.
Restore the behavior from before c8615bcd4, which inadvertently swapped the
pointers.
Bug: #9007
The byte backstop leaves many tiny OOO chunks unbounded, and the scan
only inspected the in-progress file, which an interleaved second
handle could hide. Add a chunk-count backstop (configured count, 1024
hard cap when disabled) and check every open file tx.
Ticket: #8791.
Replace the hardcoded 2 MiB gap backstop, which silently truncated
OOO file data, with the configured queue size per direction (1 GiB
hard cap when disabled), raising SMBEvent::TruncatedFileData. It runs
ahead of the chunk_left == 0 early return, and a gap reaching the
tracker tags the session gapped even when fully consumed.
Ticket: #8791.
The SMB1 WRITE_ANDX/READ_ANDX handlers fed the file tracker
unconditionally and SMB2 only checked raw in-flight occupancy. Apply
the max-{read,write}-queue-{size,cnt} checks, projected onto actual
queue growth, to both protocols' found and create-new tx branches;
on overflow raise the queue event and skip the record's remainder.
Ticket: #8791.
The chunk-continuation queue-limit rejection accounted its stream
consumption and skip from the count-based chunk promise
(chunk_left), but the parser accepts file_len < count and the
tracker only ever consumes file_len bytes. With count > file_len
the consume/skip ran past the record's actual tail into the
following RPC record and the stream desynced.
Consume/skip from the record's unbuffered tail plus the record's
XDR padding.
Ticket: #8791.
Rework the queue-limit gap backstop: fall back to a 1 GiB / 1024
chunk hard cap when the limits are disabled, run it ahead of the
chunk_left == 0 early return, and scan every open file tx in the
direction (an interleaved handle can hide a first file's queue).
TruncatedFileData is attributed to the truncated tx.
Ticket: #8791.
The chunk path (filetracker_update) enqueued per-segment data with no
queue limit. Check the limits on every enqueuing path, projected onto
actual queue growth: only OOO data at a new offset grows the queue,
and only a new OOO chunk adds a map entry. Overflow raises the queue
event on the affected tx and skips to the record boundary.
Replace the 2 MiB gap backstop with a queue-limit one (the chunk-count
cap and all-open-file scan come next), and tag the session gapped on
every gap (even one fully covered by a skip), since the backstop is
gated on that flag. Rejected and evicted tx now release their file
resources so the file logger can clean them up.
Ticket: #8791.
Replace the hardcoded 16 MiB record caps with configurable
max-{write,read}-size and observable events: an oversized WRITE
request or READ response is rejected, logged, and its payload skipped
to keep the stream in sync. 0 disables the check.
For v4 records, the issue is more complex due to the COMPOUND type:
A partial v4 COMPOUND record whose frag_len claim is huge must not be
buffered toward that claim (31-bit, attacker-controlled → OOM). The
full parser can't help: on a partial buffer it only returns Incomplete
("need more bytes"), with no information about what's coming. The scan
instead walks the buffered prefix's ops by wire extent and reads the
file op's header words alone, answering the gate's four-way question:
oversized claim detected (reject + skip to the record boundary before
any data buffers), data not yet present (buffer normally), structurall
malformed (bounded rejection), or no oversized op (fall through). It
exists because rejecting before buffering requires the claimed length,
which only a prefix walk can extract.
Ticket: #8791.
Bound the per-file-transaction OOO queue, mirroring SMB: add
max-{read,write}-queue-{size,cnt} (0 disables, default 64 MiB / 64),
raise a queue event on overflow, and skip the truncated record's
remainder from the stream.
Ticket: #8791.
An edge case existed where a lua detect script might fail to reenable
the mem limit after temporarily clearing it to allow for set up. This
would then be persistent for the lifetime of that thread.
Make sure to test these cases before the reset.
Ticket: #8805.
Uninitialised pointer dereference: lua_newuserdata() returns
uninitialised heap, so s->set contains garbage until :get()
is called. LuaDatasetAdd() then dereferences this wild pointer.
Address by memsetting the whole object.
Address string size issue: LuaDatasetAdd() fetches a pointer from
lua_tostring(L,2) but takes an independent byte-count from lua_tonumber(L,3)
without comparing it to the real string length.
Check the real string len and pass that along.
Ticket: #8807.
LuaDatasetAdd() dereferenced s->set without ensuring :get had been
called first. The userdata metatable validation it was originally
part of is already in place (Ticket: #8882); what remained is the
missing init check.
Reject :add on an uninitialized dataset. The pointer itself is
zeroed in a follow-up commit.
Ticket: #8882.
LuaFlowvarSet() accepted a script-supplied integer for the copy length
without validating it against the actual Lua string length returned by
luaL_checkstring. This could lead to OOB memory access.
This patch checks the length of the string before using it.
Ticket: #8858.
Both parse loops stopped as soon as a command or reply reached the
configured maximum length: they advanced the input cursor past the end and
broke out. When that line ended with an LF in the same stream slice it was
already complete, and the bytes behind the LF were ordinary FTP lines that
the break discarded. A segment holding "PORT <long>\r\nQUIT\r\n" lost the
QUIT: it never became a transaction, so inspection and logging never saw
it, and a hidden RETR or PORT never reached AppLayerExpectationCreate,
leaving its data connection unrecognized as ftp-data.
Remove both breaks so the loop continues to the next line. The cursor
assignment goes with them, since nothing reads it once the loop ends, and a
line with no LF still finishes the slice on its own because the getter
zeroes the remaining length.
Removing the breaks is not enough for a line that never reaches the clear.
Truncation was cleared deep inside command and reply handling, which is
skipped for an unrecognized request command, a reply the parser rejects (a
run of spaces, empty once trimmed), or an invalid EPRT/PORT that jumps to
tx_complete. The connection flag then stayed set and the getter dropped the
line behind it as this line's remainder. Snapshot and clear the flag up
front in both loops, for every line whose LF was consumed, so the following
line survives.
Issue: 8884
When the PADDED flag is set on HTTP/2 HEADERS or PUSH_PROMISE frames,
the parser reads the 1-byte Pad Length field but never strips trailing
padding bytes before passing the buffer to the HPACK decoder.
Fix by stripping trailing padding before invoking the HPACK decoder.
Ticket: #8787.
SWF depth settings were previously limited only to values representable by
uint32_t. This is not sufficient for the buffer-size calculations performed
during SWF decompression.
The decompressed output requires an additional 8-byte FWS header. The
inspection buffer also grows by doubling a uint32_t size, so a required buffer
larger than 2 GiB can cause the growth calculation to wrap.
For LZMA input, a synthesized 13-byte header to the compressed
data is used. The configured compressed-data limit must leave room for this header.
This patch applies the following upper limits:
- decompress-depth: 2147483640 bytes
(2 GiB minus the 8-byte FWS header)
- compress-depth: 4294967282 bytes
(UINT32_MAX minus the 13-byte LZMA header
Ticket: #8914
SWF FileLength was used to determine the decompressed buffer size when
decompress-depth is set to zero, but this field can be manipulated and it can
lead to evasion.
This patch treat FileLength as metadata, the length bytes are just copied, and
when decompress-depth is set to 0, a maximum value of 50MB is considered for
the decompressed buffer.
Ticket: #8855
The on-demand buffer growth introduced for SWF decompression called
lzma_decompress again from byte zero each time the output buffer filled.
Because lzma-rs can buffer up to a dictionary before writing, a single
SWF could be decoded repeatedly as the buffer grew, amplifying CPU use.
This adds a depth-bounded Rust writer that asks C to grow the inspection
buffer as decompressed bytes are emitted.
This keeps a single lzma-rs stream alive, so the input is decoded only once.
Ticket: #8829
Previously, SWF decompression guessed the final output size from FileLength,
allocated the whole buffer, and then decompressed once. A malicious SWF could
claim a 50MB output and force the per-thread inspection buffer to allocate and
retain 64 MiB before the compressed data was even checked.
Change the logic:
allocate 4 KiB, decompress, grow only when the buffer is full, and continue.
The buffer grows geometrically up to the configured output limit,
and the final inspection length is set to the number of bytes actually produced.
Also, for zlib, Z_OK does not by itself mean that decompression is complete.
It should be checked together with avail_out and avail_in,
grow the buffer when output space is exhausted, and continue the same inflate stream.
Ticket: #8829
When running with --reject-dev in autofp mode, the libnet context is
cached thread-locally and t_inject_mode is latched to LIBNET_LINK on the
first reject for an Ethernet packet. Subsequent rejects from different
interfaces that hit the fast path reuse the cached ctx but incorrectly
still call SetupEthernet(), which dereferences PacketGetEthernet() causing
a crash.
To address this check for if a packet is ethernet in all entry points.
Ticket: #8832.
The check was too weak: it allowed fh_handles far beyond what fits
in the remaining input, letting a crafted packet drive a large
Vec::with_capacity preallocation in nom7 count().
Bound fh_handles by the remaining input size and by 4096, the
maximum file layout stripe count used by the Linux kernel
(NFS4_PNFS_MAX_STRIPE_CNT).
Ticket: #8816.
On Windows the traversal logic only matched the literal "..\", but
Windows accepts "/" as well.
Fix SCPathContainsTraversal() on Windows to check both "..\\" and "../".
Ticket: #8830.
AddAndRotate() assumes the SYN queue has at least two elements when
evicting the oldest to make room for a new one. With stream.max-syn-queued
set to 1 this would not be true leading to crash.
Ticket: #8833.
Replace the BUG_ON with proper error handling so that any fopen() failure
logs a warning and skips the chunk instead of causing a crash.
Adds a few more warnings for error conditions.
Ticket: #8823.
Segment logging could create larger buffers than the thread buffer
available, which would be silently truncated. However the length passed
to libpcap was still of the non-truncated size, so this would lead
libpcap to read past the end of the buffer.
Fix by trying to expand the buffer as needed.
Ticket: #8835.
Negotiate protocol parser used an unbounded many1(complete(...))
combinator over the entire remaining packet input, discarding both
wct and bcc fields. Each NUL byte yielded one empty "dialect" entry,
causing excessive memory use.
Use bcc to limit the parsing; the dialect count is then bounded by
bcc (u16).
Ticket: #8852.
AppLayerExpectationCreate gets a locked and ref counted
IPPair from IPPairGetIPPairFromHash. On the success path it correctly
drops both with IPPairUnlock (keeping the ref for lifecycle). The else-
branch that creates per-IPPair ExpectationLists calls SCCalloc. If
that allocation fails, neither Unlock nor Release is called, leaving
the IPPair mutex permanently held and its refcount elevated.
Any subsequent worker thread that looks up the same IPPair blocks
forever in SCCMutexLock.
This patch addresses this by properly releasing the IPPair in the
allocation error path.
Ticket: #8851.
pthread_getattr_np re-initializes the attributes on use causing the
original pthread_attr_init to leak. Destroy after use to prevent the
memory leak.
Ticket: 9032
This patch distinguishes malformed DHCP options from truncated ones.
Previously, the malformed_options field was never set and parsing errors in
overloaded file and sname fields were silently discarded.
This caused malformed options to be reported as truncated or not reported at all.
Classify errors in the primary option stream and overloaded fields.
Treat failures within fixed-size overloaded fields as malformed because those
fields are already fully present.
Ticket #8874
DHCP lease, renewal, and rebinding time options contain a four-byte value,
but the parser previously ignored their declared length and always consumed
four bytes.
Reject these options unless their length is exactly four to
prevent parser desynchronization and incorrect keyword matches.
Ticket #8873
DHCP option 0 is a one-byte padding field, but it was parsed as a generic TLV.
This consumed the following option code as its length and could prevent later
options from being inspected.
Handle PAD explicitly so parsing resumes at the next option.
Ticket #8819
Since the PostgreSQL protocol doesn't impose any clear limits to how
many responses a backend could send, add one to the app-proto parser.
Using 21-64 as the accepted values, and 64 as the default:
The Start-up phase may have up to 17-18 ParameterStatus messages, plus
AuthOk, BackendKeyData and ReadyForQuery: 20, with extra room to be on
the safer side: 21
And 64 as upper limit, because as we currently summarize query
responses, we already don't get flooded by DataRow and Copy responses
messages, so we don't expect to see that many responses in a single
transaction for the pgsql, with the current implementation.
Ticket: #7263
Pgsql logger would only open a ParameterStatus message array and only
close it when a different type of message was seen (following an
expected pgsql transaction style). However, if no different type of
message followed the ParameterStatus message, the array remained open,
leading to an unbalanced `}` count, and poisoning the whole JSON log,
from that point on.
Now, if we finish the responses logging loop without reaching another
type of response, close the array, regardless.
Ticket: #9012
parse_row_description and parse_consolidated_data_row read a field_count
from the message body and use that to iterate a slice, without checking
whether the count actually fits the body.
A backend-controlled attack could lead to fewer fields logged, or
stalling of the parser to-client direction for the duration of the flow.
A malformed_response event is set for pgsql, if either of those
message's inner parsers has a field_count that can't be reconciled with
the body slice.
A new pgsql.response log field is added to indicate this, too:
pgsql.response.row_description_malformed, a boolean. This wasn't added
to the consolidated data row message, per its summarization nature.
The row_description unit test buffer declared a length one byte longer
than its three fields occupy. The old code silently discarded that
surplus byte; the new one reports it, so the buffer was corrected.
Ticket: #8885
Samaresh Kumar Singh
Philippe Antoine
Juliana Fajardini
Giuseppe Longo
William Bland
Victor Julien
Jeff Lucovsky
Jason Ish