Several C loggers are registered through a cast to
EveJsonSimpleTxLogFunc, which has an incompatible signature. Clang's
function sanitizer reports this as a call through a pointer to an
incorrect function type.
Change these loggers to the generic callback signature and register them
without casts.
Ticket: 9226
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
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
CopyCommandLine allocated a buffer for the whole request line, stripped
the trailing whitespace and returned the shorter length. That shorter
length is what the caller stores in tx->request_length and what
FTPTransactionFree later hands to FTPFree, so one byte for every stripped
whitespace character is never subtracted from ftp_memuse. The memory
itself is released -- SCFree ignores the size it is given -- but the
counter keeps climbing for the life of the process, and a client that
pads its commands with spaces can push it up with every command it sends.
Once the counter reaches app-layer.protocols.ftp.memcap, FTPCheckMemcap
refuses any allocation that would cross it, and request buffers are the
first to be refused: the transaction is still created, but tx->request
stays NULL, so command_data is missing from the log and rules that
inspect it stop matching. Whether a new flow can still allocate its state
depends on how much room is left below the cap. With no memcap
configured, the symptom is an ftp.memuse counter that is permanently
wrong.
Strip the whitespace before allocating, so the size accounted for is the
size returned to the caller:
- CopyCommandLine shortens line->len first, then allocates and copies
that length.
- A line that is nothing but whitespace returns without allocating. It
used to get a buffer that was stored on the transaction and freed with
a length of zero, leaving the whole allocation on the counter.
- When the allocation fails, line->len has already been shortened, so
port_line, the transfer file name and the too-long-line check see the
same length there that they see when it succeeds.
- FTPParserTest13 covers the accounting and the stored request;
FTPParserTest14 covers the line the parser uses afterwards.
Issue: 8848
Ticket: 8592
Fixes: 5ddd808e9b ("ftp: don't halt the flow when raising
too_many_transactions")
In the case we receive a big chunk of TCP data, we end up
creating much more than max-tx transactions, and have
quadratic complexity on this packet, even if all
these transactions get cleaned up at the end of the processing
of this packet.
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
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
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
Ticket: 8001
Will allow algorithms such as ac to use the MpmCtx, especially
the number of patterns, to preallocate some structures.
For detect engine PatternMatchThreadPrepare, create a dummy
MpmCtx out of all the MpmCtx in the DetectEngineCtx,
and get the max value for the number of patterns
The first version of the event set done=true on state->curr_tx and
returned NULL when the limit hit. curr_tx is usually the active
command just received — marking it done mid-request throws off
response matching, and subsequent commands in the flow stop getting
logged.
SMB behavior is mirrored here:
Walk the tx list, find the oldest tx that isn't done, mark
it done and tag it with the event, then fall through and
create the new tx so the flow parsing continues. One stale tx gets
reaped per overflow so memory stays bounded.
Issue: 8489
Other parsers (SMB, POP3, ENIP) already raise a too_many_transactions
event when a flow exceeds its per-flow transaction limit. FTP had
the limit wired up through app-layer.protocols.ftp.max-tx but just
dropped the excess transaction on the floor with a "FTP does not set
events yet..." TODO, so no event was raised when the limit was
reached.
Add the FtpEventTooManyTransactions variant, a stock rule at
sid 2232002, and raise the event from FTPTransactionCreate when the
live-tx count goes past ftp_config_maxtx.
Issue: 8489
Ticket: 6813
So, in the case we have a ftp-data flow with data to client as
expected, but after to_client side completes (with timeout for
example), the client sends some data to server,
In this case, we want to check first the direction, and bail out,
then do the assertion check about the finished state.
Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.
Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.
Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.
Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.
FTP parser can handle multiple command lines per direction. Appropriate calls
to trigger raw stream inspection have been added on succesful parsing of each
request line and response line.
Bug 7742
Use the Rust logic to parse FTP response lines with the goal to support
multi-buffer matches better.
A side effect is that the completion codes are no longer strings; the
schema update reflects this.
Issue: 4082
Issue: 4082
Move the configuration file handling to Rust.
These changes will no longer terminate Suricata when there's an invalid
value for ftp.memcap. Like earlier Suricata releases, an error message
is logged "Invalid value <value> for ftp.memcap" but Suricata will no
longer terminate execution. It will use a default value of "0" instead.
To optimize detection, and logging, to avoid going through
all the live transactions when only a few were modified.
Two boolean fields are added to the tx data: updated_tc and ts
The app-layer parsers are now responsible to set these when
needed, and the logging and detection uses them to skip
transactions that were not updated.
There may some more optimization remaining by when we set
both updated_tc and updated_ts in functions returning
a mutable transaction, by checking if all the callers
are called in one direction only (request or response)
Ticket: 7087
Ticket: 6390
This can happen with keyword filestore:both,flow
If one direction does not have a signature group with a filestore,
the file is set to nostore on opening, until a signature in
the other direction tries to set it to store.
Subsequent files will be stored in both directions as flow flags
are now set.
Jason Ish
Philippe Antoine
Jeff Lucovsky
Victor Julien
Shivani Bhardwaj
Lukas Sismis