An empty SETTINGS frame (an ACK, or a SETTINGS without entries) opened
the settings array and closed it without items, producing a record the
eve schema rejects (settings has minItems 1). Skip the frame instead; a
tx whose only content is an empty SETTINGS frame is then not logged at
all, matching the empty headers handling.
Exposed by the flow-end flush of IDS-dropped flows: the dropped flow in
the drop-protocol-change test carries such a frame.
AppLayerParserGetTxEndState() replaces the remaining copies of the
transaction end state computation in the firewall tx setup, the
app-layer-state and app-layer-event keywords and the completion check.
One tolerated a missing tx data pointer, the others asserted or
dereferenced it, so they could disagree.
Move DetectTxCompleted() above its first use while there, so it no
longer needs a forward declaration, and shorten its comment.
Ticket: #8944.
DetectEngineInspectGenericList returned NO_MATCH for every keyword miss,
also when the tx had moved past the phase the engine is registered at.
Unlike the buffer engines it never produced a terminal no match, so a
firewall LTE rule using such a keyword (tls.version on a hook) stayed
provisional and was accepted for the rest of the session.
Make a miss terminal at P + 1. This engine also backs the lua
on-request/on-response hooks and the tls_cert_*, dnp3/dnp3_ind and
ftpdata/ftp mode keywords, so a plain IDS rule using them keeps its
verdict once the tx has moved past the registered phase.
Export the parser's end state computation as AppLayerParserGetTxEndState()
here, so the version keyword can use it; the remaining call sites are
converted later.
Ticket: #8944.
banner_wait_eol only recorded that a banner line of 256 bytes or
more had arrived without EOL; it was not an active phase. The
direction kept reparsing the open line in place, so a rule or
firewall policy pointing at the state acted on a parked, not an
active, direction.
Move the condition to the event channel it already had (the
one-shot ssh.long_banner event) and key the continuation off an
explicit banner_published latch (set at the banner publish
sites): a direction with an open line stays in the banner state,
the line is consumed greedily until the EOL arrives, and the line
completion takes the normal banner-to-kex transition.
The state scale becomes dense: banner(0) kex(1) session(2),
completion done(3). request_banner_wait_eol /
response_banner_wait_eol no longer load: aliasing them to banner
would silently turn a long-banner rule into an any-banner rule.
Firewall rules that acted at the removed state must move to the
ssh.long_banner event or the banner state; and since the progress
gap between banner and kex is gone, the default-policy sweep no
longer drops a clean flow at the server banner.
Ticket: #9046.
A flow that hit an unrecoverable ssh parse error (invalid banner
or invalid record) was never visible in the eve ssh output: the
log condition only admitted a successful handshake (both
directions at kex or session), and the tx logger is one-shot, so
the successful-handshake log fired before a later failure could
be reported and the failure was then silent - the reason was
only visible through the app-layer events.
The log condition is now failure-only: the failure latches, so
the object is emitted exactly once - at the failure, carrying
the error field (invalid_banner / invalid_record) and the state
field (the state the failure occurred in - banner, kex, or
session if a later record fails after NewKeys) - and successful
flows are logged at the flow-end flush, where the engine logs
unconditionally. Logging on the success condition mid-flow would
consume the one-shot tx log and drop a later failure, so it is
not done. The state field is emitted only alongside the error.
The object may be error + state only, when no banner was
parsed. A unit test pins the condition for a failed, a
banner-only and a both-at-kex flow (the failure admits the log
on the failing delivery); the user guide and eve reference doc
commits that follow carry the field docs.
That flow-end flush is skipped for flows that end without a
further app-layer update, though: in IPS/firewall mode a drop
disables the app layer (StreamTcpCheckFlowDrops ->
StreamTcpDisableAppLayer), after which no app-layer update
reaches the tx loggers, and a bypassed flow gets no flow-end
packet either. A successful ssh record - e.g. one that only saw
the banners - was therefore never emitted for those flows. The
hook is the app-layer end: on the transition to app-layer EOF,
StreamTcpDisableAppLayer raises FLOW_APP_LAYER_FLUSH_PENDING,
and FlowUpdateState raises it on the transition to a bypassed
state; OutputTxLog runs the tx loggers once for the next packet,
also when that packet itself has no app-layer update and is not
marked established (the disabling packet returns from the
stream engine before that), and flow-worker clears the flag when
it frees the app layer of a bypassed flow. The flush is
deliberately not a flow-end flush: when the flush is the only
reason to run the loggers, eof is not forced, so each logger's
condition decides and no other parser starts logging records it
suppressed; a disabling packet that does carry an app-layer
update keeps the existing EOF behaviour. SSHTxLogCondition
admits an IPS drop or a bypass, so the pending record is emitted
there; an IDS drop does not end the app layer, so the condition
stays failure-only - a later failure still wins, and the success
record is emitted by the flow-end flush, which the flow timeout
path now also runs for IDS-dropped flows. The unrelated
schema-sort ordering of the new state keys is fixed up as well.
Ticket: #9046.
A banner line that completes while parked in banner_wait_eol now
takes the direction to kex: the banner data was already parsed at
entry, the completion only advances the state. Before, the
direction skipped the kex state entirely and jumped to session at
the newkeys record.
The tx is marked updated at parse entry, as the base parser did,
before the dispatch: every delivery - including the stash and
header-fragment cases that parse_record handles and publish
nothing - is decided by the state rules and the firewall
default-policy sweep (default: drop) rather than left to the
firewall default-accept, as an un-updated tx is not evaluated
mid-flow. The parse entry does not pre-check those cases:
parse_record returns the same AppLayerResult for them, and the
record_left arithmetic must stay in sync in one place only.
Ticket: #9046.
An SSH direction that fails to parse (invalid banner or invalid
record) now keeps the state it was in when the failure occurred:
that state is the last progress the direction reports, and the
failure is exposed through the app-layer events
(ssh.invalid_banner / ssh.invalid_record) and the per-direction
error field tracked here.
The failing direction is frozen at its state: the failure is
unrecoverable, so the parse entry points short-circuit on the
error flag - no further parsing, no state change - and
post-failure garbage can neither advance nor regress it. The
failure site returns a parse error, so the engine ends app-layer
parsing for the whole flow.
The no-inspection/bypass flag arming now runs on the session
transition itself, shared by the complete-record and
incomplete-header arms: a NewKeys record whose body arrives in a
later parser call (segment boundary after the 6-byte record
header) reached the session state without arming the flags, so
the encrypted stream was parsed as records. The arming keeps the
peer check (a merely-failed peer does not arm them). A unit test
pins the split-record arming, including BYPASS_READY in bypass
mode.
Ticket: #9046.
The old ssh state scale was coupled across the two directions: the
terminal finished state was only reached after both sides sent NewKeys,
and the progress accessor clamped every per-direction phase to a
shared value. A rule hooking the old top state therefore meant "the
flow completed", not "this direction completed the handshake", and
there was no room above the terminal state for a true completion
marker.
Replace the scale with per-direction phases:
banner(0) -> banner_wait_eol(1) -> kex(2) -> session(3)
NewKeys now sets session on the packet's own direction only, banner
completion sets kex, and the progress accessor is identity: each
direction reports its own raw phase. Add the registered completion
state done(4), never assigned during stream parsing: it stays out
of both name tables, so a rule referencing it fails to load, the
hook listing omits it and the firewall policy loader does not
consult it; the progress accessor reports it only on flows
disrupted by stream depth or async reassembly.
The legacy state names (in_progress, banner_done, finished) no
longer resolve through the name table (pre-production - breaking
changes allowed); the name-table unit test pins the rejection.
The old firewall policy state keys are added to the rename hint
table (like the TLS migration), so a config using them fails
loudly with the replacement named instead of degrading to a bare
warning. The non-prefilter engine-name registration now guards
the null id-to-name result (a hook-only rule at the completion
progress previously formatted a null name into the engine
name).
Add unit tests for the state name table (new names, rejected legacy
names, the unhookable completion state, id-to-name) and for the
per-direction session state.
The eve log condition compares the per-direction states with
>= rather than ==: a direction at session has necessarily passed
kex, and a single delivery can take a direction
banner -> kex -> session, so a final {session, kex}
pair must still admit the log (done is never assigned, so >=
changes only the kex branch).
Ticket: #9046.
block_scrutinee now warns on a block-wrapped if let scrutinee, since
the block changes when temporaries drop. The file config reads in the
nfs, smb and http2 gettxfiles handlers were wrapped in a pointless
single-expression block; remove it.
A response line carrying a code but no message, such as "221 \r\n" sent by
some servers in reply to QUIT, was logged with no completion_code, and with
the code itself in the reply field:
{"command": "QUIT", "reply": ["221"], "reply_received": "yes"}
parse_response_line split caused problems on no response message and prevented good logging of completion code
Ticket: #8966
The internal type set for datasets of type IP is IPv6, however, the
expectation is that it'll support IPv6, standard IPv4 as well as IPv4 in
IPv6 notation. A regression happened during the conversion of datasets
parsing to rust wherein standard IPv4 was rejected with IP type. Fix
that by handling IPv4 standard notation in IPv6 handler.
Fixes: bc79300dfa ("datasets: move initial file reading to rust")
Ticket: 9006
Ticket: 9096
In case of a 100-continue response, we used to
- parse the response line, set progress to headers
- parse headers, set progress back to line
This caused FPs as the engine expects the progress to just increase
so that it skips signatures that did not match yet, and whose
progress is less than the tx progress.
Solution is to keep progress at line when we see a 100 response,
until we get to the body, or after a next response line which
is not 100.
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
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.
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.
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
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.
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.
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
Avoids pushing multiple log messages, when the consumer will
only set one app-layer event for the tx
Also, this is a warning, not an error, as we continue parsing.
Ticket: 8867
Ticket: 8800
Thus avoiding unbounded recursion
Before that, parse_payload could call
parse_security_association_payload which could call back
parse_payload if a type in the list was SecurityAssociation
But a Security Association only accepts the type Proposal in its
payload, see RFC 2408: section 3.5
There was another recursive path with parse_payload calling
parse_proposal_payload which called back parse_payload
And a Proposal only accepts the type Transform in its payload, see
RFC 2408: section 3.6
Fixes: 9aeeac53 ("pgsql: remove probe_ts function")
When the probe_ts function was removed in 9aeeac532, the request gap
handling got inverted, preventing re-sync of the app-proto from the
frontend side.
This could lead to detection evasion after a gap.
Ticket: #8822
Ticket: 8755
The config parameter was only taken into account from the
settings frame HEADER_TABLE_SIZE
Emits an event http2.header_table_overflow when we have the first
header that we cannot keep in Suricata's memory due to the
configured limit.
parse_t123_tpkt runs the X.224 and X.223/MCS sub-parsers over the
length-bounded tpkt payload, but they use streaming nom parsers.
An inner length field overrunning the payload, such as an oversized
CS_CORE block, escaped as Err::Incomplete although the tpkt was fully
present.
This patch wraps the child parsers in complete() so Incomplete from within the
bounded payload becomes an Error.
The tpkt is then returned as an unrecognized Raw child, which the caller skips,
and parsing resumes at the next tpkt.
Ticket #8838
When an incomplete SSH_MSG_KEXINIT record has a body of at least
SSH_MAX_REASSEMBLED_RECORD_LEN bytes and hassh is enabled, record_left
was overwritten with the full body length even though the function
returns ok() and thus consumes the body bytes already present in the
input. The next call then over-skipped by that many bytes into the
following record, desynchronising the parser for that direction so
hassh was never computed and ssh.hassh keywords could not match.
This patch keeps the record_left computed from the bytes actually consumed in
the oversized path, and only use the full body length in the path that
returns incomplete, where the body bytes are delivered again.
Ticket #8837
MQTT5 logs user properties wherein a user can send any metadata
over the wire in the form of key-value pairs.
MQTT logger used these key-value pairs directly to populate the common
"properties" field. But we do not want to use arbitrary json keys
(which may contain arbitrary characters)
1. A fixed new child object to the common "properties" called
"user_properties" which carries a list of dicts.
2. Each key-value pair received as a part of the user properties is now
saved as:
"properties": {
...
"user_properties": [{"key": "mykey", "value": "myvalue"}]
...
}
instead of:
"properties": {
...
"key": "value"
...
}
Ticket: 8785
Keys were not encoded as json UTF-8 strings.
This allowed json injection/corruption when an attacker
could control the key and put special JSON chars in it
such as double quote. Encode the keys when needed.
Ticket: 8999
Ticket: 9003
When a single tx is getting inspected by rules having
http.response_header keyword both with a transform and without
a transform, the code went like this :
Http2ThreadMultiBuf structure contains a list (Vec) of buffers
(Vec<u8>) and owns these Vec<u8> (they are not owned by the tx).
For the first list_id, we computed each buffer for a http2 header
and put them in these Http2ThreadMultiBuf structure.
But for the second list_id, the code began by freeing the memory
of these http2 headers, and reallocated new buffers (Vec<u8>)
and recomputed each buffer.
After these rounds, which may happen in DetectRunPrefilterTx,
the buffers might need further checks, hence the use-after-free.
Now, the code does not free and allocate a new buffer, it just rewrites
the old one, as is done for HTTP1.
CopyIn and CopyOut message parsers read the length field, but did not
use that information to parse the rest of the message, instead, parsing
as many columns as indicated in the columns field, without ever
checking against the informed length. This opened the door for
attackers to fabricate messages that were never sent by the server,
as well as to obfuscate following valid backend messages, in case the
columns value was bigger than the actual message (up to ~128KiB).
Those parsers also had a message length parsing check off-by-one, fixed
in this patch.
When a malformed CopyIn or CopyOut response is seen, it is logged as
previously, with a new field added to indicate whether it is malformed
or not.
Ticket: #8863
parse_request and parse_response were using the parser state for
transaction bookkeeping.
Instead, use the message types themselves.
This change means that message types that ended up not being added to
the responses array, before (due to not advancing the parser state), now
are pushed and thus can be logged.
** Noticeable effect: AuthenticationSSPI, Notice and Notification
messages, previously missed due to falling into the None state progress
branch, could now get logged.
** To prevent affecting existing setups that don't expect these, since
they weren't logged before, we refrain from pushing them into the
responses queue, for now.
Leave state progress for state machine keeping.
Bug: #8886
So far, the properties parsed across various different types of MQTT
messages was unbounded and could be exploited. Add a limit of 255 on the
maximum number of properties that any message type can store.
Additionally, raise an event for too many properties indicating an
anomaly in the network traffic.
Ticket: 8839
Introduce a maximum SAN limit of 4k instead of returning a u16 casted
count of TLS X.509 SubjectAltNames, saturate the count to make sure it
stays within the 4k bounds without truncating the existing count value.
Ticket: 8864
ENIP parser makes use of nom's complete macros which by design expect
full data to be passed along to them. In case there is incomplete data
sent to them, they result in an error. This disables the ENIP parser and
further detection and could happen with an incomplete/fragmented
request/response.
Use AppLayerResult API to ask for more data from the stream engine in
case the stream slice currently at hand does not match the length
parameters received from the header or is not long enough to parse the
header itself.
Ticket: 8809
In "tx:answers()" don't attempt to push each OPT as its own string,
instead concatenate into one string like we do with TXT.
The previous method was broken and didn't work anyways, so we're not
breaking something that previously worked.
Ticket: 8843
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
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.
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
Philippe Antoine
Victor Julien
Amir Boussejra
William Ling
Shivani Bhardwaj
Jason Ish
Juliana Fajardini
Giuseppe Longo
Jeff Lucovsky