SCFTPFreeResponseLine gated the release of the code buffer on a null
test of the response pointer, so the code buffer had no check of its
own. Neither check could fire: parse_response_line sets both pointers
from Box::into_raw on a boxed slice, which never returns null, and C
code obtains an FTPResponseLine only from SCFTPParseResponseLine. The
comment on the code field claimed the pointer may be null; a line with
no status code gets an empty buffer with code_length 0 instead.
Remove both null checks, note why they are unnecessary, and correct the
field comment.
Ticket: 8803
parse_response_line allocates the code buffer with Box::into_raw on a
boxed slice, but SCFTPFreeResponseLine released it by rebuilding a
Vec<u8> with capacity code_length. Vec::from_raw_parts accepts any
global-allocator pointer whose size and alignment match, and both paths
compute the same layout for a byte slice, so the old free was sound. It
differed only from how the response buffer is released.
Release the code buffer through Box::from_raw on a slice pointer, so
both buffers are freed the same way they were allocated.
Ticket: 8803
In case a keyword should work in firewall mode, with firewall rules
only. The engine errors out if threat detection rules use the given keyword.
Part of
Ticket #9041
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.
Philippe Antoine
Jason Ish
Jeff Lucovsky
Juliana Fajardini
Victor Julien
Amir Boussejra
William Ling
Shivani Bhardwaj
Giuseppe Longo