Commit Graph

12307 Commits (8d62ca8fb0cb2bfb34c6b6fdfbbe1b75ae710777)
 

Author SHA1 Message Date
Philippe Antoine da824d8252 detect: config checks alstate before getting tx
Ticket: 4972

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

(cherry picked from commit dccf2e4c30)
4 years ago
Philippe Antoine 483194893e detect: not an iponly signature if it needs app-layer
Ticket: 4972

This may happen with `config` keyword which is postmatch,
but may require a transaction

(cherry picked from commit 0cba561fec)
4 years ago
Philippe Antoine d4d8b4c5b3 detect: makes config keyword really require a flow
Ticket: 4972

Completes commit c3a220647

DETECT_CONFIG is added as DETECT_SM_LIST_POSTMATCH and not
as DETECT_SM_LIST_MATCH as other keywords handled in SignatureCreateMask

(cherry picked from commit 00da0d3420)
4 years ago
Philippe Antoine 272f082b85 detect: only apply ConfigApplyTx with app-layers
Ticket: 4972

Otherwise, it makes no sense to look for a tx...

(cherry picked from commit c3a220647b)
4 years ago
Jason Ish eeb8c17e0f 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.

Ticket #5034

(cherry picked from commit 27679a12aa)
4 years ago
Jason Ish fa04c1bc57 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.

Ticket #5034

(cherry picked from commit 0623ada24d)
4 years ago
Jason Ish 947cb09e5b 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.

Ticket #4849

Backported manually by jason.ish@oisf.net.

(cherry picked from commit 464ff80c6a)
4 years ago
Philippe Antoine c6a0709dc8 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

(cherry picked from commit dd32238667)
4 years ago
Philippe Antoine 0ab108e120 dnp3: check Base64Encode return value for logging
So that NULL pointers do not get logged

Ticket #4849
4 years ago
Philippe Antoine bda9f8abac util: right parenthesises for base64 macro
So that BASE64_BUFFER_SIZE(3) == 5 instead of 7
4 years ago
Philippe Antoine 8498999c6a tftp: use destate
And avoids memory leaks on it

Ticket #4848
4 years ago
Jason Ish 6075cc157b 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.
4 years ago
Victor Julien 20b379d92a smb: fix read queue exceeded event and rules 4 years ago
Victor Julien 788d8abea3 smb: log max read/write sizes
(cherry picked from commit 90d4b8e438)
4 years ago
Victor Julien 4d53fa78e5 smb/rules: add rules for new events
(cherry picked from commit b0354437d5)
4 years ago
Victor Julien 6e5b7199cf doc/smb: add resource limits section
(cherry picked from commit 976748b777)
4 years ago
Victor Julien 65f24b2e84 smb2: validate negotiate read/write max sizes
Raise event if they exceed the configured limit.

(cherry picked from commit fc9b65d8d3)
4 years ago
Victor Julien 8510031e6c smb2: allow limiting in-flight data size/cnt
Allow limiting in-flight out or order data chunks per size or count.

Implemented for read and writes separately:

app-layer.protocols.smb.max-write-queue-size
app-layer.protocols.smb.max-write-queue-cnt
app-layer.protocols.smb.max-read-queue-size
app-layer.protocols.smb.max-read-queue-cnt

(cherry picked from commit 4be8334c9e)
4 years ago
Victor Julien 5b38b97181 filetracker: track total queued data (in_flight)
As well as expose number of chunks.

(cherry picked from commit 2c5ad8858e)
4 years ago
Victor Julien 502db40240 smb2: add options for max read/write size
Add options for the max read/write size accepted by the parser.

(cherry picked from commit 5bcc4162f7)
4 years ago
Victor Julien 9f969e2545 smb2: track max read/write size and enforce its values
(cherry picked from commit f28888513a)
4 years ago
Victor Julien 2ba9ad53eb smb: minor function cleanup
Remove used argument from `filetracker_newchunk()`. We're not
using fill_bytes with smb.

(cherry picked from commit 594acec5dc)
4 years ago
Victor Julien 02c3bd00fa filetracker: make FileChunk private
(cherry picked from commit c7a474c725)
4 years ago
Philippe Antoine f3a6d15034 mqtt: fix consumed bytes computation for truncated msg
Ticket: 5268
(cherry picked from commit 3b13008c1b)
4 years ago
Sascha Steinbiss 084b16a63b mqtt: raise event on parse error 4 years ago
Jason Ish 4c2d543022 mqtt: ensure we do not request extra data after buffering
This is a minimal backport of 5618273ef4
to address ticket 5018.

Uses the "complete" version of take instead of the macro which is thre
streaming variant.

Ticket #5018
4 years ago
Jason Ish b589b05efd github-ci: pin checkout actions plus other fixups
Pin checkout action plus other fixups from master to deal with changes
to the action.
4 years ago
Jason Ish fca9c69bc7 smb: rules for messages in the wrong direction
(cherry picked from commit 1e65324940)
4 years ago
Jason Ish e55fef32d9 smb: handle records in the wrong direction
If an SMB record is seen in the wrong direction, set an event on the PDU
frame and don't process the record in the state.

No error is returned, so the next record will be processed.

(cherry picked from commit 2341f47755)
4 years ago
Jason Ish e63795543f smb: expose smb1 request/reply flags with a method
Adds `.is_request()` and `.is_reply()` to check if a SMB record flags
say the message is a request or a reply.

(cherry picked from commit 09e2d3b216)
4 years ago
Jason Ish 12a8415326 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.

(cherry picked from commit 7b659489c8)
4 years ago
Philippe Antoine 83490d4e3c tftp: StringToAppProto case
So, fuzz_applayerparserparse_tftp will fuzz tftp

(cherry picked from commit c9d664b0a0)
4 years ago
Jason Ish fd57cf76bf 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.

Ticket #5201

(cherry picked from commit 8d1e4a1d0b)
4 years ago
Shivani Bhardwaj 0ddf39344e detect/dataset: cleanup dead code
(cherry picked from commit 7366396011)
4 years ago
Shivani Bhardwaj 5e084d4daa 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.

(cherry picked from commit 6d2a2a0731)
4 years ago
Victor Julien b8305b1108 flow: fix and simplify locking
Since:

9551cd0535 ("threading: don't pass locked flow between threads")

`MoveToWorkQueue()` unconditionally unlocks the flow. This allows simpler
locking handling, including of tcp reuse flows.

The simpler logic also fixes a scenario where TCP reuse flows got "unlocked"
twice, once in `FlowGetFlowFromHash()` and once in `MoveToWorkQueue()`.

Bug: #5248.
Coverity: 1494354.
(cherry picked from commit 57533d3e47)
4 years ago
Jeff Lucovsky e0834b3047 log/stack: Propagate original signal
Issue: 4550

This commit modifies the "stack trace on signal" to propagate the
original signal received instead of always raising SIGABRT.

(cherry picked from commit a3443845fb)
4 years ago
Jeff Lucovsky 0e2d76f836 config: Make libunwind use configurable for 6.0
Issue: 4973

This commit makes stack-trace on fault configurable by
adding "--enable-libunwind" as a configure option.

By default, or if "--enable-libunwind=no" is specified, the libunwind
library will not be configured.

When "--enable-libunwind=yes" is specified, libunwind will be used iff
it can be found in one of the standard library locations.
4 years ago
Jeff Lucovsky 1306dbf268 doc/yaml: Signal-termination option description
(cherry picked from commit 93842aa14a)
4 years ago
Jeff Lucovsky 529e69b27b 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.

(cherry picked from commit 7f0f463b64)
4 years ago
Jeff Lucovsky c1506ebde4 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.

(cherry picked from commit 163f70be9d)
4 years ago
Jeff Lucovsky 9f68fc946a error: Add error code for sig-related diagnostics
This commit adds an error code for the diagnostic code used for
diagnostic messages following unexpected termination due to signals..

(cherry picked from commit 501c870a2c)
4 years ago
Jeff Lucovsky 285f57861e configure.ac: Support libunwind configuration
This commit adds support for enabling libunwind -- a library that can be
used to display stack information.

Libunwind is enabled and used by Suricata if present during
configuration.  A diagnostic message is displayed if libunwind
cannot be found.

(cherry picked from commit 303dd29b50)
4 years ago
Victor Julien 866d0872be nss: use 'atexit()' to cleanup
This avoids ASAN to report leaks in case of fatal errors in tests.
4 years ago
Arne Welzel 22c47396fd 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:

    hash_size=1000
    flowmgr_number=3
    range=333

    instance  ftd->min  ftd->max
    0         0         333
    1         334       666
    2         667       1000

    Rows not covered: 333, 666

(cherry picked from commit 8ef066318d)
4 years ago
Philippe Antoine 16c52db465 ssl: first pass limit when allocating buffer for certificates
With this check, on the first packet of a certificate presenting
a length of 16Mbytes, we only allocate up to 65Kb

When we get to the point where need more than 65Kb, we realloc
to the true size.

With this check, it makes it more expensive for an attacket to use
this allocation as a way to trigger ressource exhaustion...

(cherry picked from commit 862e84877f)
4 years ago
Philippe Antoine 384611da1a fuzz: use fuzzing confyaml for protodetect target
As is done for other targets,
so that all app-layer protocols are enabled,
even the ones disabled by default such as enip

And resets protocol detection every time we try
so that probing_parser_toserver_alproto_masks are fresh.

(cherry picked from commit 09c84d0c26)
4 years ago
Philippe Antoine 5af5b125ee 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...

(cherry picked from commit 4247605d87)
4 years ago
Victor Julien ce69f79f6a 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.

(cherry picked from commit b336882008)
4 years ago
Steven Ottenhoff 9b1557b7a7 pppoe: fix protocol field length variation
Detect when protocol field is not a 16 bit field.
Added tests to prove logic

Ticket: 4810
(cherry picked from commit 6bf2117056)
4 years ago