Victor Julien [Fri, 22 Apr 2022 16:27:15 +0000 (18:27 +0200)]
stream: improve last_ack validation check
If a packet after the initialization would come with ACK flag set
but a ACK value of 0, the last_ack tracking could get confused. Fix
this by not checking for 0 but instead checking if the ACK flag
has been seen.
Philippe Antoine [Mon, 22 Nov 2021 10:30:08 +0000 (11:30 +0100)]
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.
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.
When switching from SMTP to TLS, and getting HTTP1 instead of
expected TLS, and HTTP1 requesting upgrade to HTTP2, we do not
overwrite the alproto_orig value so as not to have type confusion
in AppLayerParserStateProtoCleanup
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 [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 [Thu, 14 Apr 2022 19:16:55 +0000 (13:16 -0600)]
github-ci: set safe directory before reset
While the latest checkout action does set the "safe.directory"
parameter, it doesn't appear to stick for the following "git fetch", so
call this command again.
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
mqtt: ensure we do not request extra data after buffering
This addresses Redmine bug #5018 by ensuring that the parser
never requests additional data via the Incomplete error, but to
raise an actual parse error, since it is supposed to have all
the data as specified by the message length in the header already.
Some unittests used SCMalloc for allocating new Packet the unittests.
While this is valid, it leads to segmentation faults when we move to
dynamic allocation of the maximum alerts allowed to be triggered by a
single packet.
This massive patch uses PacketGetFromAlloc, which initializes a Packet
in such a way that any dynamic allocated structures within will also be
initialized.
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.
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:
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.
devguide: drop use of mscgen script in builds/make
Currently, it seems easier to upload the diagram images to git than to
try to make the image generation script work with out of the tree builds
and other corner cases.
This means, however, that one must activelly remember to update msc
diagram files, run the script and re-add new png files, if those ever
need to be updated. To raise awareness to that, a watermark was added
to the diagram images.
Also removed configuration steps that added mscgen as dependency
(locally and for workflow builds and readthedocs).
Moved devguide dir into userguide dir.
Since the devguide is now incorporated as the last chapter of the
userguide, removed build and configuration files from the devguide
dir, as these are no longer needed.
projects / thirdparty / suricata.git / log
