The functions in util-ioctl.c use an AF_INET datagram socket to
perform interface ioctl operations. Most of them returned silently
when socket creation failed, while GetIfaceRSSQueuesNum() logged only
a warning.
Report socket creation failures as errors in all of these functions.
Such failures can indicate a system-level problem, such as file
descriptor exhaustion, and otherwise make startup and interface
reconfiguration failures difficult to diagnose.
Signed-off-by: Petr Vaganov <petrvaganoff@gmail.com>
The filedata inspect engine returned NO_MATCH for every non-matching
file, also when the tx had already moved past the engine's progress.
For streaming registrations such as the http2 request body this meant a
firewall LTE rule never became definitive, so the fail-closed default
policy was never applied.
Return CANT_MATCH_FILES once the tx progress moved beyond the engine,
mirroring the existing no file case. The per file state reset keeps the
rule matchable if the tx grows another file.
Ticket: #9122.
-fsanitize=implicit-conversion flags the frag_offset + data_len
narrowing into the uint16_t frag_end. For both IPv4 and IPv6, compute
frag_end after the too-large check so the sum always fits, and make the
narrowing an explicit cast.
For IPv6 this moves the existing IPV6_MAXPACKET check above the
frag_end assignment. It used to sit at the end of the branch, after
frag_end was already computed. Nothing reads frag_end in between, so
behaviour is the same. Oversize fragments are still dropped with
IPV6_FRAG_PKT_TOO_LARGE, and the wrap can no longer happen, which
matches the IPv4 path.
Also drop the vestigial int32 casts when assigning the uint32_t
tracker id.
Ticket: 8232
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.
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
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
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.
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.
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
Treat bad link-header CRCs and impossible link lengths as recoverable
errors and attempt to resynchronize.
Also consume a response frame whose trusted length is too short for the
required response headers, so later responses remain available.
Ticket: 8980
Ticket: 8916
Avoid type confusion and possible overread when calling
AppLayerParserGetStateProgress
DetectEngineInspectMultiBufferGeneric callback may be called with
the inner DNS tx of a HTTP2/DOH2 tx, so we should use dns as its
protocol
The per-thread threshold decision cache (limit/both thresholds tracked
by_src/by_dst, IPv4) grew without bound: every distinct tracked address
allocated an item plus a hash-table bucket, while housekeeping reclaims
at most two entries per whole second.
Address this by:
Cap the cache at detect.thresholds.cache.max-entries entries per
thread (default 256; valid range 256-1048576, out-of-range values
fail startup); on overflow evict the entry with the earliest expiry
before inserting
Account cache memory in the new detect.thresholds.cache.memuse
counter: each entry (item plus bucket node) is charged on insert and
released on eviction, expiry and on thread teardown, and the fixed
per-thread charge (thread context, hash table struct and the eagerly
allocated bucket pointer array) is charged at cache init and released
symmetrically on teardown, so the gauge reflects the cache memory in
use and does not accumulate stale charges when threads are recreated,
e.g. unix-socket PCAP file jobs
Size the cache hash table to the entry cap (next power of two, minimum 256)
at thread init so lookups stay O(1) up to the cap.
Fail startup when the threshold table cannot be initialized, instead
of continuing with a NULL table.
Ticket: #8956.
DetectBytejumpValidateNbytesOnly() only performed signed upper-bound
checks but had no lower-bound guard. When byte_jump used a
variable-supplied nbytes value derived from byte_extract/byte_math, an
extracted field with bit 31 set arrived as a negative int32_t that
passed validation and the ptr+len boundary check.
The downstream (uint16_t)nbytes truncation in the binary extract branch
then produced a small positive count and ByteExtractUint64() over-read
up to 7 bytes past the end of the inspection buffer.
Address this by adding a nbytes >= 0 check.
Ticket: #8831.
Fix an infinite loop in SCInspectionBufferCheckAndExpand where doubling
a uint32_t new_size can overflow if min_size is too large. This causes
detection worker threads to spin at 100% CPU. This is a very unlikely
scenario, but this patch hardens the logic to be defensive.
Ticket: #8834.
DetectFileInspect() walks the per-signature SigMatchData array with
a pointer (smd) that is incremented inside the inner keyword-matching
loop. Because smd is the function parameter, this mutation persists
across iterations of the outer for file loop and there is no reset
of the cursor between files.
With a single file this works correctly. In any transaction that carries
two or more File objects every subsequent file is evaluated starting at
the already-advanced smd instead of the beginning of the keyword chain,
leading to skipping of keywords.
This patch addresses the issue by using a local iterator instead of
using the function parameter directly, so every file sees the full keyword
list.
Ticket: #8815.
PrefilterPostRuleMatch() repopulates det_ctx->pmq with flowbits:isset
candidates after a matching flowbits:set rule is found. The subsequent
append loop writes directly into det_ctx->tx_candidates[] without first
checking that the array has room.
The buffer was initially sized for prefilter results only not accounting
for post-rule additions. When array_idx already equals tx_candidates_size,
the writes go past the end of the heap-allocated block - a heap buffer
overflow followed by qsort() over OOB memory, causing a crash (DoS).
Addressed by adding a capacity check + expand before the append loop,
matching the existing pattern at all other candidate append logic.
Ticket: #8866.
Ticket: 8973
Before that, the update to DoH2 happened before http1 state was
freed, leading to a type confusion.
In case of a HTTP1->HTTP2 upgrade, if the client sends a DOH2
request, Suricata went to change again the flow's alproto
with SCAppLayerForceProtocolChange
But SCAppLayerForceProtocolChange did not take into account the
fact that we were doing a HTTP1->HTTP2 upgrade, and the previous
HTTP1 state was not freed yet.
Now SCAppLayerForceProtocolChange checks FlowChangeProto
and update alproto_orig only if we were not doing a proto change
already, as alproto_orig is used to know which type of state
to free.
Ticket: 8775
Avoids that every flow that is the same except for spi
ends up in the same bucket, and that this bucket grow too much
since FlowCompareESP makes them different flows
The truncation flag lived in FtpState, where the line getter set it and
the two callers cleared it once FtpLineState.lf_found said the line had
ended. The callers reuse one FtpLineState for every line in a slice, and
only the LF branch ever set lf_found, so a complete line left it set for
whatever came after. An over-long line behind a complete one in the same
slice therefore had its truncation state cleared, its remainder was parsed
as ordinary traffic, and a "PASS s3cret" sitting in that remainder became
a transaction of its own.
The flag could just as easily be left set. The request loop reached its
clear only for a line that matched an FTP command, continuing to the next
line otherwise, so an over-long run of "Z" left the flag standing and the
next command to arrive was discarded as that line's remainder. The
response loop's clear sat three ifs deep, inside blocks entered only when
the reply parsed, so a line that SCFTPParseResponseLine rejects -- a run
of spaces is empty once trimmed -- left it standing too.
The getter also raised the flag for a line whose LF it had just consumed.
Nothing of that line remained to discard, so the caller had to clear it
straight back.
Truncation is a property of the line, so FtpLineState now carries it. The
getter sets line.truncated for the line it hands back, and the FtpState
flag stays set only where truncation genuinely spans slices: the over-long
line that arrives without an LF. The callers read line.truncated and no
longer touch the shared state, so lf_found goes away entirely.
Issue: 8859
When a command or reply runs past the configured maximum length, the
parser keeps what fits, reports it truncated, and discards the remainder
of that line as it arrives. Only the remainder should go, but the parser
was discarding the whole stream slice that the line's LF landed in.
A slice holding "<remainder>\r\nQUIT\r\n" carries a complete QUIT behind
the remainder, and a response slice carries whatever reply comes next. The
recovery branch zeroed the remaining length and returned an error, which
ended the caller's parse loop, so those lines were discarded along with
the remainder they followed. They never became transactions, so nothing
inspected them and nothing logged them, which means a command can be
hidden by placing it after an over-long one.
Consume through the first LF, clear the truncation state there, and pick
up the next line in the same slice. The scan becomes a loop so the
remainder goes through the ordinary path, including when it is over-long
itself; the run-out-of-input check becomes the loop condition.
Raised in review of the 8859 fix.
Issue: 8859
When a line exceeds the configured maximum length and contains no LF,
FTPGetLineForDirection emitted a window starting at the beginning of the
stream slice rather than at the unconsumed remainder. If a complete
command had already been taken from the same slice, that window was
shifted left by the consumed count: it began inside the previous command
and stopped short of the end of the long one.
The command MPM match, the per-transaction request copy and the port_line
capture all ran over those stale bytes, so a long command hidden behind a
short one in the same segment was classified and logged as the previous
command. The response direction had the same problem.
The LF-found branch already derives the line from input->buf + o_consumed;
do the same here, and advance consumed so the cursor still describes the
slice it was handed.
Issue: 8859
Philippe Antoine
Petr Vaganov
Victor Julien
Samaresh Kumar Singh
William Bland
Jeff Lucovsky
Giuseppe Longo
Jason Ish
Shivani Bhardwaj