Ticket: 8694
Otherwise, a flow full of small compression bombs is too slow
to process.
When the threshold is reached, decompression is skipped for the
rest of the flow.
(cherry picked from commit 392b6aee29)
Ticket: 8725
So that multiple HTTP2 DATA frames with EndOfStream flag set,
do not make the buffer grow, while processing it each time,
resulting in quadratic complexity
(cherry picked from commit 1731805967)
Ticket: 8629
When we are in async-oneside mode, we see only one direction
of the traffic, and should not wait for the other direction
before cleaning up a transaction.
(cherry picked from commit 7ec9d72d28)
new_tx() now refuses to create a transaction when the list is already at
SMB_MAX_TX, returning None instead of a transaction. Every creation path --
the new_*_tx helpers and their callers across smb1/smb2/dcerpc/session/
files/ioctl -- propagates that, so no single input can create more than the
limit, including a compound SMB2 request that chains many PDUs in one
record. When the list is full the parser puts the flow into an error state
and stops processing it.
This replaces the previous force-completion of old transactions, which did
not reliably bound the list and could leave transactions unreclaimable on
asymmetric flows. The now-unused tx_index_completed bookkeeping is removed.
Issue: 8629
(cherry picked from commit c8f68b0e4f)
Ticket: 8558
Do not accept an IPv6 as an IPv4 (even if the hash is likely
different in the first place)
Do the check also for ESP, and ebpf
(cherry picked from commit bc41dcc854)
Ticket: 8649
Fully resets all the fields before tackling an ecapsulated message
to avoid evasion, due to the encoding of the upper file
leaking into the next one...
(cherry picked from commit 4985eb9dae)
Body lengths were stored as u16, so a SIP body of 65536 bytes truncated
body_len to 0 and the RequestBody/ResponseBody frame was never created,
letting body content evade inspection. Widen the framing fields to u32.
Ticket #8582
(cherry picked from commit 499995bf0e)
Fix Flow Manager error when flows were not being timed out
in case flow.hash-size < 10.
This happened because calculation of variable rows_per_sec in
flow-manager.c rounded the value to zero.
This commit ensures that variable rows_per_sec is at least 1.
Ticket: 8710
(cherry picked from commit b8d7f894be)
Tx ID handling did not take the required + 1 into account.
From a report:
RDP can skip cleanup because its id convention does not match the
generic Rust iterator. The generic iterator in applayer.rs returns
tx.id() - 1, and cleanup trusts that id when calling StateTransactionFree
in app-layer-parser.c. RDP registers that iterator in rdp.rs, but
RdpTransaction::id() returns the stored id unchanged in rdp.rs, while
free_tx also compares against the raw stored id in rdp.rs. For a single
freeable RDP tx with stored id 1, the iterator returns C id 0; cleanup
calls free_tx(0), nothing is removed, then has_next == false allows
min_id to advance to total_txs in app-layer-parser.c. That leaves the
tx live but now below min_id, so later cleanup will not revisit it.
This patch brings the handling in line with the other parsers.
Bug: #8717.
(cherry picked from commit 9555e3add6)
In firewall mode, a accept:hook or accept:tx needs to lead to a accept
packet when the action is applied to the last TX. For this the code
relied of the `DetectTransaction::is_last` field, where the assumption
was that there would always be an inspection on the last transaction.
This assumption was wrong however, as transactions can be skipped for a
few reasons: not updated, fully inspected, unidirectional for the other
direction. This would cause the accept not be applied to the packet,
leading to a default drop.
The reason this wasn't noticed before is that until now the work had
focused on protocols that used sequential transactions (http/tls),
and/or short lived sequential unidir transactions (dns)
This patch addresses the issue by making a simple assumption: if the
last available transaction in the main detection loop is skipped, it
means it has been accepted before. Therefore we can apply the "accept
packet" logic in this case.
Bug: #8698.
(cherry picked from commit 9e31a21aba)
Treat dyn_port 0 as an absent value so numeric comparisons do not
match FTP transactions without a negotiated data port.
Ticket: #8392
(cherry picked from commit e560db9d1f)
Support STOU on the FTP data channel.
With STOU a filename is optional. If no filname is provided, use
"<stou>". The server does respond with the filename to use, but there
can be a race condition in getting this filename and setting up file
storage before data starts flowing.
Ticket: #8664
(cherry picked from commit f59960d42b)
Support the APPE (append) command on the ftp data channel. Like STOR,
APPE uploads file content on the data channel.
Ticket: #8664
(cherry picked from commit e65bf4c905)
Register ftp.dynamic_port at the FTP transaction completion progress
instead of progress 0. This allows ftp.dynamic_port to be used in
ftp:response_complete.
Ticket: #8392
(cherry picked from commit 41cad78506)
Register ftp.command_data with the FTP request completion progress so it
can be used by firewall rules on the ftp:request_complete hook.
Ticket: #8392
(cherry picked from commit a6ce31d50d)
Register ftp.command with the FTP request completion progress so it can
be used by firewall rules on the ftp:request_complete hook.
Ticket: #8392
(cherry picked from commit bf63c574d3)
Ticket: 8659
If a client sends a SOTR/RETR command before doing a PASV/PORT
command, ther server may reply
425 Use PORT or PASV first.\r\n
and allow the client to continue sendinf other commands
So, Suricata should not put the whole flow into error, it just
sets an event, and continues to parse further
(cherry picked from commit 52490b8c26)
Ticket: 8642
Do not allocate too much by using the config value, when the flash
file does not require that much data anyways
(cherry picked from commit d9fae18432)
Mark the direction into RDP transactions at creation time,
so the tx carries SKIP_INSPECT for the direction it is
never seen in, matching DHCP and the other single-direction
parsers. This lets cleanup free completed transactions and
stops a tx from being inspected (and alerting) twice, once per
direction.
RDP bounds its transactions to connection setup and stops
parsing once bypass_parsing is set.
Issue: 8621
(cherry picked from commit 5af13e3eb0)
DHCP is a stateless parser where each datagram is its own standalone,
single-direction transaction. It was creating transactions with
AppLayerTxData::new(), which leaves both SKIP_INSPECT bits clear, so the
engine treats every tx as still needing inspection in both directions.
For a flow that only ever carries one direction (broadcast DHCP, or a
relay seeing one side), the never-observed direction's inspect bit can
never be set, so AppLayerParserTransactionsCleanup() never frees the tx.
The per-flow transaction Vec then grows without bound and every packet
re-scans the whole list, giving O(n^2) CPU and unbounded memory on a
busy DHCP aggregation point.
Use AppLayerTxData::for_direction() like every other stateless parser
(DNS, SNMP, NTP, IKE, KRB5, MQTT, QUIC, SIP, WebSocket, bittorrent-dht)
so the tx carries SKIP_INSPECT for the direction it will never be seen
in. This lets cleanup free completed transactions and also stops the tx
from being inspected (and alerting) twice, once per direction.
Issue: 8621
(cherry picked from commit 3f86c56c80)
Revert issue 8619 fix in favor of a more general change that
sets INSPECTED_T{S,C} correctly on passed flows. This makes clean no
longer need the FLOW_ACTION_PASS special case to free those
transactions.
Issue: 8621
(cherry picked from commit 30d4edf9eb)
Issue: 8628
AppLayerParserSetTransactionInspectId() had an inverted guard when
tagging completed transactions as inspected: it set the INSPECTED flag
only on txs that already had it, so a finished tx was never newly
marked.
On a normal flow DetectRunTx() sets the same flag and masks the
mistake. On a passed flow -- a `pass` rule or a pass-the-flow exception
policy -- detection is skipped and this function is the only thing that
marks txs inspected. With the inverted guard the transactions are never
flagged, AppLayerParserTransactionsCleanup() never frees them, and the
per-flow transaction list grows without bound while each packet
re-scans it, giving O(n^2) cleanup cost.
This is an unintended regression from 834378ff88.
(cherry picked from commit 006e21c1cc)
The ftpdata_command would never match a RETR in the to_server direction,
or a STOR in the to_client direction, this effectively makes the
ftpdata_command applicable to passive mode only.
Instead, remove the directionality on the ftpdata_command. Its "meta"
information passed from the control channel and isn't really
directional. I think the idea was to abstract away active/passive from
the rule writer, but unfortunately doesn't allow the ftpdata_command
keyword to work in the more strict firewll mode.
Ticket: #8663
(cherry picked from commit 2fe4bb2872)
In FTP, the first transaction is a banner from the server. Skip TS
inspection on this transaction.
Allows firewall mode to get past this transaction and onto the next.
Ticket: #8662
(cherry picked from commit d346e469c5)
Reading a pcap from /dev/stdin or a named pipe currently fails with "failed to get first packet timestamp. pcap_next_ex(): -1" because InitPcapFile calls setvbuf on the FILE* underlying the pcap handle after libpcap has already consumed the pcap header. On a non-seekable fd glibc cannot recover from that and the next read returns -1.
Detect non-regular files via fstat and skip setvbuf in that case so the read keeps working on pipes, fifos and stdin.
Accept pcap-file.buffer-size values of 0, which disables setvbuf buffering as an explicit opt-out, or PCAP_FILE_BUFFER_SIZE_MIN (4 KiB) to PCAP_FILE_BUFFER_SIZE_MAX (64 MiB). Treat any non-zero setvbuf return value as an error, not just negative values.
When pcap-file.buffer-size fails to parse, retain the default buffer size instead of falling through and setting it to 0. The branches are now mutually exclusive so only one of the parse-error, accepted, or out-of-range messages is logged.
Update the user guide: --pcap-file-buffer-size now documents valid values of 0 (disables setvbuf buffering) or 4 KiB to 64 MiB, and pcap-file.rst notes that 0 is the opt-out for non-seekable sources such as stdin and named pipes.
Bug: #8464.
(cherry picked from commit 565e138754)
Ticket: 8600
CreateServiceA doc states for example :
> If the path contains a space, it must be quoted so that it is
correctly interpreted.
Also fixes strlcat usage and check return value to error out on
truncated path
(cherry picked from commit ac1b3cc1ef)
This reverts commit 923ad6af77.
Several issues have been reported:
1. non-deterministic thread to queue mapping (privately reported)
2. race condition causing IPS mode to loose packets, or get stuck
completely, ticket 8667.
Ticket: #8667.
(cherry picked from commit 2b19f86ee6)
Ticke: 8651
Uses it in place when we dereferenced the value straight away
after checking SCConfGet result but not its value
(cherry picked from commit 6bb271cee9)
Philippe Antoine
Jeff Lucovsky
Giuseppe Longo
Adam Kiripolsky
Victor Julien
Jason Ish
Jeff Lucovsky
Samaresh Kumar Singh
Shivani Bhardwaj
Andreas Dolp