avoid quadratic complexity from removing the first element
and copying all the contents a big number fo times.
Ticket: #5909
(cherry picked from commit 9adb59bcdb)
Stamus team did discover a problem were a signature can shadow
other signatures.
For example, on a PCAP only containing Kerberos protocol and where the
following signature is matching:
alert krb5 $HOME_NET any -> any any (msg:"krb match"; krb5_cname; content:"marlo"; sid:3; rev:1;)
If we add the following signature to the list of signature
alert ssh $HOME_NET any -> any any (msg:"rr"; content:"rr"; flow:established,to_server; sid:4; rev:2;)
Then the Kerberos signature is not matching anymore.
To understand this case, we need some information:
- The krb5_cname is a to_client keyword
- The signal on ssh is to_server
- Kerberos has unidirectional transaction
- kerberos application state progress is a function always returning 1
As the two signatures are in opposite side, they end up in separate
sig group head.
Another fact is that, in the PCAP, the to_server side of the session
is sent first to the detection. It thus hit the sig group head of
the SSH signature. When Suricata runs detection in this direction
the Kerberos application layer send the transaction as it is existing
and because the alstate progress function just return 1 if the transaction
exists. So Suricata runs DetectRunTx() and stops when it sees that
sgh->tx_engines is NULL.
But the transaction is consumed by the engine as it has been evaluated
in one direction and the kerberos transaction are unidirectional so
there is no need to continue looking at it.
This results in no matching of the kerberos signature as the match
should occur in the evaluation of the other side but the transaction
with the data is already seen has been handled.
This problem was discovered on this Kerberos signature but all
the application layer with unidirectional transaction are impacted.
This patch introduces a flag that can be used by application layer
to signal that the TX should not be inspected. By using this flag
on the directional detect_flags_[ts|tc] the application layer can
prevent the TX to be consumed in the wrong direction.
Application layers with unidirectional TX will be updated
in separate commits to set the flag on the direction opposite
to the one they are.
Ticket: #5799
Remove the second occurrence of tree_id logging which appears to
always be a duplicate of the first tree_id logged, even though they
come from different data structures.
Issue: 5811
(cherry picked from commit 67baab573b)
If a file (read/write) SMB record has padding/trailing data
after the buffer being read or written, and that Suricata falls
in one case where it skips the data, it should skip until
the very end of the NBSS record, meaning it should also skip the
padding/trailing data.
Otherwise, an attacker may smuggle some NBSS/SMB record in this
trailing data, that will be interpreted by Suricata, but not
by the SMB client/server, leading to evasions.
Ticket: #5786
(cherry picked from commit 233ab11148)
When Suricata handles files over SMB, it does not wait for the
NBSS record to be complete, and can stream the payload to the
file... But it did not check the consistency of the SMB record
length being read or written against the NBSS record length.
This could lead to an evasion where an attacker crafts a SMB
write with a too big Length field, and then sends its evil
payload, even if the server returned an error for the write request.
Ticket: #5770
(cherry picked from commit c1b7befb18)
After a gap in a file transaction, the file tracker is truncated. However
this did not clear any stored out of order chunks from memory or stop more
chunks to be stored, leading to accumulation of a large number of chunks.
This patches fixes this be clearing the stored chunks on trunc. It also
makes sure no more chunks are stored in the tracker after the trunc.
Bug: #5781.
(cherry picked from commit a24d7dc45c)
As of Rust 1.66 with strict mode enabled, a for loop over an option is
now an error. Replace the last occurrence of this pattern with an "if
let" statement.
Fuzzing highlighted an issue where a command sequence on the same file
id triggered a logging issue:
file data for id N
close id N
file data for id N
If this happened in a single blob of data passed to the parser, the
existing file tx would be reused, the file "reopened", confusing the
file logging logic. This would trigger a debug assert.
This patch makes sure a new file tx is created for the file data
coming in after the first file tx is closed.
Bug: #5567.
(cherry picked from commit 45eb038e63)
As an SMB2 async response does not have a tree id, even if
the request has it.
Per spec, MessageId should be enough to identifiy a message request
and response uniquely across all messages that are sent on the same
SMB2 Protocol transport connection.
So, the tree id is redundant anyways.
Ticket: #5508
(cherry picked from commit e94920b49f)
It was not enough to set Cursor position to 0,
also its inner Vec should be cleared.
This way, a new input gets written at the beginning of the
Cursor and its inner Vec...
Ticket: #5691
(cherry picked from commit 086b28da3d)
When having many transactions in a single parsing call...
Fix has overhead of having one more field in the mqtt state.
Completes commit a8079dc978
Ticket: #5399
(cherry picked from commit e160917bcf)
Allows users to find balance between completeness of decoding
and increases resource consumption, which can DOS suricata.
(cherry picked from commit e42094f238)
Ticket: 4530
So, that we do not get DOS by quadratic complexity, while
looking for a new pkt_id over the ever growing list
of active transactions
(cherry picked from commit a8079dc978)
This is a backport of ea1d03f8e3 by Pierre
Chifflier adapted for 6.0.x.
> The `count` combinator preallocates a number of bytes. Since the value
> is untrusted, this can result in an Out Of Memory allocation.
> Use a maximum value, large enough to cover all current implementations.
Ticket: #5448
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.
Ticket #5034
(cherry picked from commit 27679a12aa)
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.
Ticket #5034
(cherry picked from commit 0623ada24d)
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.
Ticket #4849
Backported manually by jason.ish@oisf.net.
(cherry picked from commit 464ff80c6a)
tianjinshan
Jason Ish
Philippe Antoine
Eric Leblond
Philippe Antoine
Victor Julien
Jeff Lucovsky
Jeff Lucovsky