The input data received in DATA and BDAT command modes can be huge and
could have important data, like a legit huge email. Therefore, exempt
these from the line buffering limits which were introduced to regulate
the size of lines that we buffer at any point in time.
As a part of this patch, anything that comes under DATA or BDAT is
processed early without buffering as and when it arrives. The ways of
processing remain the same as before.
Shivani Bhardwaj [Mon, 14 Feb 2022 11:23:52 +0000 (16:53 +0530)]
smtp: fix indefinite buffering if no LF in line
Issue
-----
So far, with the SMTP parser, we would buffer data up until an LF char
was found indicating the end of one line. This would happen in case of
fragmented data where a line might come broken into multiple chunks.
This was problematic if there was a really long line without any LF
character. It would mean that we'd keep buffering data up until we
encounter one such LF char which may be many many bytes of data later.
Fix
---
Fix this issue by setting an upper limit of 4KB on the buffering of
lines. If the limit is reached then we save the data into current line
and process it as if it were a regular request/response up until 4KB
only. Any data after 4KB is discarded up until there is a new LF char in
the received input.
Cases
-----
1. Fragmentation
The limit is enforced for any cases where a line of >= 4KB comes as diff
fragments that are each/some < 4KB.
2. Single too long line
The limit is also enforced for any cases where a single line exceeds the
limit of buffer.
Jason Ish [Wed, 6 Apr 2022 21:38:35 +0000 (15:38 -0600)]
ftp: truncate command data that is too long
FTP control commands will be buffered forever until a new line is seen,
this can lead to memory exhaustion in Suricata.
To fix, set an upper bound, 4096 bytes on the size of the command that
is saved in the transaction. The input continues to be parsed to find
the end of the command so the parser can continue to move onto the next
command.
The result is that the command data in the transaction is truncated,
which also shows up in the ftp transaction logs.
This value is configurable with the max-line-length field in the ftp
app-layer.protocols section.
As FTP doesn't have events at this time, add a new fields to eve-log
that specificy if the request, or the response has been truncated.
As is done in detect-lua-extensions.
We can have a flow with alproto unknown, no state, and therefore
cannot run AppLayerParserGetTx which could try to run a NULL
function
Jason Ish [Mon, 28 Feb 2022 22:48:34 +0000 (16:48 -0600)]
dns: don't parse a full request during probe if not enough data
If there is more data than a header, but not enough for a complete DNS
message, the hostname parser could return an error causing the probe to
fail on valid DNS messages.
So only parse the complete message if we have enough input data. This is
reliable for TCP as DNS messages are prefixed, but for UDP its just
going to be the size of the input buffer presented to the parser, so
incomplete could still happen.
Jason Ish [Tue, 1 Feb 2022 21:44:43 +0000 (15:44 -0600)]
dns: better error handling when parsing names
The DNS name parser will error out with an error even if the
error is incomplete. Instead of manually generating errors,
use '?' to let the nom error ripple up the error handling chain.
The reason this wasn't done in the first place is this code
predates the ? operator, or we were not aware of it at the time.
This prevents the case where probing fails when there is enough data to
parse the header, but not enough to complete name parser. In such a case
a parse error is returned (instead of incomplete) resulting in the
payload not being detected as DNS.
Jason Ish [Thu, 31 Mar 2022 18:45:07 +0000 (12:45 -0600)]
smb: protocol detection on pattern without midstream
To recognize a protocol, Suricata first looks for
patterns, which can be confirmed by a probing parser.
If this does not work, Suricata can try to run
some probing parsers on some ports.
This is the case for SMB.
This commit makes handling the confirming and the probing
paser differently even if they share much code.
The confirmation parser knows that a pattern has been found.
So, it must not do the midstream case of looking for this
pattern in the whole buffer, but only check it at the beginning.
But it must reverse direction if needed.
Philippe Antoine [Mon, 29 Nov 2021 09:59:10 +0000 (10:59 +0100)]
ftp: do not set alproto if one was already found
Ticket: 4857
If a pattern such as GET is seen ine the beginning of the
file transferred over ftp-data, this flow will get recognized
as HTTP, and a HTTP state will be created during parsing.
Thus, we cannot override directly alproto's values
This solves the segfault, but not the logical bug that the flow
should be classified as FTP-DATA instead of HTTP
Jason Ish [Wed, 20 Apr 2022 17:10:35 +0000 (11:10 -0600)]
app-layer: better warning message when enabling by default
The warning message suggests that the configuration section doesn't
exist if when it does, but the "enabled" flag is not set. Clarify the
warning message that the enable status is not set.
Jason Ish [Mon, 18 Apr 2022 16:32:25 +0000 (10:32 -0600)]
smb: fix smb2 header flag parsing
The bits were being parsed in the order they're displayed in Wireshark,
rather than the order they were being seen on the wire, resulting in
direction and async being 0 more often than they should be.
Instead of bits, take the 4 bytes as an le_u32 and just use bit masks to
extract what we need into a struct, I think its easier to reason about
this way when comparing to the Microsoft documentation.
Jason Ish [Tue, 22 Mar 2022 15:46:45 +0000 (09:46 -0600)]
detect-content: error on single char hex pairs
Fix parsing of content like "|aa b cc|" which was parsed as "|aa bc|"
without error or warning. This will now fail out, requiring all hex
values to be 2 chars.
Shivani Bhardwaj [Fri, 28 Jan 2022 20:17:17 +0000 (01:47 +0530)]
detect/dataset: fix space condition in rule lang
If there is a space following a keyword that does not expect a value,
the rule fails to load due to improper value evaluation.
e.g. Space after "set" command
alert http any any -> any any (http.user_agent; dataset:set ,ua-seen,type string,save datasets.csv; sid:1;)
gives error
[ERRCODE: SC_ERR_UNKNOWN_VALUE(129)] - dataset action "" is not supported.
Fix this by handling values correctly for such cases.
Jeff Lucovsky [Thu, 17 Jun 2021 13:07:29 +0000 (09:07 -0400)]
logging/diag: Enable stacktrace diagnostic if config'd
This commit adds a signal handler for SIGSEGV when configured. The
signal handler emits a one line stack trace using SCLogError. The intent
is to provide diagnostic information in deployments where core files are
not possible.
The diagnostic message is from the offending thread and includes the
stack trace; each frame includes the symbol + offset.
Jeff Lucovsky [Thu, 17 Jun 2021 13:04:56 +0000 (09:04 -0400)]
logging: Stacktrace on signal term setting
This commit adds a configuration setting to enable a stack trace message
if Suricata receives a signal that terminates execution, such as
SIGSEGV, SIGABRT.
Arne Welzel [Sat, 12 Feb 2022 16:49:07 +0000 (17:49 +0100)]
flow-manager: fix off-by-one in flow_hash row allocation
The current code doesn't cover all rows when more than one flow manager is
used. It leaves a single row between ftd->max and ftd->min of the next
manager orphaned. As an example:
Philippe Antoine [Tue, 25 Jan 2022 20:10:37 +0000 (21:10 +0100)]
smtp: check if we have a current transaction
Ticket: 4948
This is not the perfect solution, but it prevents to trigger
the assert, and keep the assert.
A better solution would need to create transaction from
the reponse parsing, in case a later command was buffered and
not answered. But this would not be enough as NoNewTx prevents
the creation of a new transaction for RSET...
Victor Julien [Fri, 25 Mar 2022 10:17:23 +0000 (11:17 +0100)]
smb1: apply close to direction
Instead of closing files in both direction when receiving a close request,
close only toserver files for the request and close toclient on receiving
a response.
Jeff Lucovsky [Thu, 6 May 2021 13:49:55 +0000 (09:49 -0400)]
proto: Remove dependency on /etc/protocols
This commit eliminates the dependency on /etc/protocols and equivalent
on other platforms by using a static table of IANA assigned protocol
values (names, description).
Jeff Lucovsky [Fri, 11 Feb 2022 14:02:39 +0000 (09:02 -0500)]
threads: Honor per-thread stack size setting
Issue: 4550
This commit adjusts the per-thread stack size if a size has been
configured. If the setting has not been configured, the default
per-thread stack size provided by the runtime mechanisms are used.
This commit documents the new per-thread stack-size setting. Some
systems have a small default value that is not suitable for Suricata's
multi-threaded architecture and adjustment may be required.
Jason Ish [Thu, 20 Jan 2022 18:08:33 +0000 (12:08 -0600)]
logging: change ownership of application log if needed
When running with privilege dropping, the application log file
is opened before privileges are dropped resulting in Suricata
failing to re-open the file for file rotation.
If needed, chown the application to the run-as user/group after
opening.
Jason Ish [Thu, 20 Jan 2022 17:40:24 +0000 (11:40 -0600)]
startup: initialize run as user info sooner
Initialize the run-as user info after loading the config, but
before setting up logging (previously it was done while initializing
signal handlers). This will allow the log file to be given the
correct permissions if Suricata is configured to run as a non-root
user.
>>> CID 1499365: (UNINIT)
>>> Using uninitialized value "infstream.total_out" when calling "inflate".
98 int result = inflate(&infstream, Z_NO_FLUSH);
99 switch(result) {
100 case Z_STREAM_END:
101 break;
102 case Z_OK:
103 break;
>>> CID 1499365: (UNINIT)
>>> Using uninitialized value "infstream.total_out" when calling "inflate".
98 int result = inflate(&infstream, Z_NO_FLUSH);
99 switch(result) {
100 case Z_STREAM_END:
101 break;
102 case Z_OK:
103 break;
*** CID 1499363: Error handling issues (CHECKED_RETURN)
/src/util-file-swf-decompression.c: 97 in FileSwfZlibDecompression()
91
92 infstream.avail_in = (uInt)compressed_data_len;
93 infstream.next_in = (Bytef *)compressed_data;
94 infstream.avail_out = (uInt)decompressed_data_len;
95 infstream.next_out = (Bytef *)decompressed_data;
96
>>> CID 1499363: Error handling issues (CHECKED_RETURN)
>>> Calling "inflateInit_(&infstream, "1.2.11", 112)" without checking return value. This library function may fail and return an error code.
97 inflateInit(&infstream);
98 int result = inflate(&infstream, Z_NO_FLUSH);
99 switch(result) {
100 case Z_STREAM_END:
101 break;
102 case Z_OK:
Victor Julien [Mon, 21 Feb 2022 19:30:35 +0000 (20:30 +0100)]
nfs3: fix partial write record handling
Comment from Jason Ish: This was a bit more than a cherry pick as some
of the modifications weren't directly portable to the nom macros. So
instead bring in some of the nom function based parsers as they work
fine with nom 5.
Victor Julien [Sun, 20 Feb 2022 09:00:48 +0000 (10:00 +0100)]
nfs/rpc: update full record parsers to be more exact
Instead of 'take'ing all data for the RPC prog_data and then
letting the higher level parsers figure out which part to use
take the exact amount.
Comments from Shivani Bhardwaj: This is was more than a cherry pick and
some of the modifications were not directly portable to the nom macros.
So, parsers were changed to make sure the functionality remained same while
making the transition to nom5 while keeping the diff minimal.