Commit Graph

12507 Commits (07639c96f25c9280871a41abc21a8eda5cf45539)
 

Author SHA1 Message Date
Victor Julien 07639c96f2 radix: add debug validation to assist scan-build
util-radix-tree.c:595:34: warning: Access to field 'stream' results in a dereference of a null pointer (loaded from field 'prefix') [core.NullDereference]
        if ((temp = (stream[i] ^ bottom_node->prefix->stream[i])) == 0) {
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
util-radix-tree.c:717:30: warning: Access to field 'stream' results in a dereference of a null pointer (loaded from field 'prefix') [core.NullDereference]
        if (SC_RADIX_BITTEST(bottom_node->prefix->stream[differ_bit >> 3],
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
./util-radix-tree.h:27:34: note: expanded from macro 'SC_RADIX_BITTEST'
 #define SC_RADIX_BITTEST(x, y) ((x) & (y))
                                 ^
2 warnings generated.

(cherry picked from commit fa5acc1743)
3 years ago
Victor Julien 88b305b539 detect/sigorder: assist scan-build
Bug: #3152.
(cherry picked from commit b625aa9748)
3 years ago
Victor Julien d2375ee7d2 detect/sigorder: remove unused struct fields
(cherry picked from commit 03e0a60f96)
3 years ago
Victor Julien 47b25329aa detect: fix scan-build warnings
detect-engine-address.c:1140:17: warning: Use of memory after it is freed [unix.Malloc]
            r = DetectAddressCmp(ag, ag2);
                ^~~~~~~~~~~~~~~~~~~~~~~~~
detect-engine-address.c:1169:17: warning: Use of memory after it is freed [unix.Malloc]
            r = DetectAddressCmp(ag, ag2);
                ^~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.

detect-engine-port.c:1161:9: warning: Use of memory after it is freed [unix.Malloc]
        DetectPortPrint(ag2);
        ^~~~~~~~~~~~~~~~~~~~
1 warning generated.

Bug: #3150.
Bug: #3151.
(cherry picked from commit 000064de7d)
3 years ago
Victor Julien 37925fe2bd mpm/ac-bs: work around scan-build warnings
util-mpm-ac-bs.c:482:32: warning: Result of 'malloc' is converted to a pointer of type 'uint16_t[256]', which is incompatible with sizeof operand type 'uint16_t' [unix.MallocSizeof]
        ctx->state_table_u16 = SCMalloc(ctx->state_count *
                               ^~~~~~~~
./util-mem.h:35:18: note: expanded from macro 'SCMalloc'
 #define SCMalloc malloc
                 ^~~~~~
util-mpm-ac-bs.c:524:32: warning: Result of 'malloc' is converted to a pointer of type 'uint32_t[256]', which is incompatible with sizeof operand type 'uint32_t' [unix.MallocSizeof]
        ctx->state_table_u32 = SCMalloc(ctx->state_count *
                               ^~~~~~~~
./util-mem.h:35:18: note: expanded from macro 'SCMalloc'
 #define SCMalloc malloc
                 ^~~~~~
2 warnings generated.

Bug: #3148.
(cherry picked from commit c8694634af)
3 years ago
Victor Julien 763833c217 mpm/ac: work around scan-build warnings
util-mpm-ac.c:531:32: warning: Result of 'malloc' is converted to a pointer of type 'uint16_t[256]', which is incompatible with sizeof operand type 'uint16_t' [unix.MallocSizeof]
        ctx->state_table_u16 = SCMalloc(ctx->state_count *
                               ^~~~~~~~
./util-mem.h:35:18: note: expanded from macro 'SCMalloc'
 #define SCMalloc malloc
                 ^~~~~~
util-mpm-ac.c:575:32: warning: Result of 'malloc' is converted to a pointer of type 'uint32_t[256]', which is incompatible with sizeof operand type 'uint32_t' [unix.MallocSizeof]
        ctx->state_table_u32 = SCMalloc(ctx->state_count *
                               ^~~~~~~~
./util-mem.h:35:18: note: expanded from macro 'SCMalloc'
 #define SCMalloc malloc
                 ^~~~~~
2 warnings generated.

Bug: #3148.
(cherry picked from commit ee683a7074)
3 years ago
Victor Julien 77e60e3368 mpm/hs: fix scan-build warning
util-mpm-hs.c:340:20: warning: Potential leak of memory pointed to by 'p' [unix.Malloc]
        p->sids[0] = sid;
        ~~~~~~~~~~~^~~~~
1 warning generated.

Incorrect error handling could lead to a memory leak.

(cherry picked from commit ec84ba1a3c)
3 years ago
Victor Julien 333021e670 decode: suppress scan-build warning
(cherry picked from commit 59ca5cc655)
3 years ago
Victor Julien 16cadd7e59 suricata: work around scan-build warnings
suricata.c:691:17: warning: Value stored to 'bits' during its initialization is never read [deadcode.DeadStores]
    const char *bits = "<unknown>-bits";
                ^~~~   ~~~~~~~~~~~~~~~~
suricata.c:692:17: warning: Value stored to 'endian' during its initialization is never read [deadcode.DeadStores]
    const char *endian = "<unknown>-endian";
                ^~~~~~   ~~~~~~~~~~~~~~~~~~
2 warnings generated.

(cherry picked from commit c13a6fa73c)
3 years ago
Victor Julien 27ef9a96af scan-build: treat as debug validate
(cherry picked from commit bcf8187443)
3 years ago
Jason Ish 00a9edae3e detect-fast-pattern: remove unused var alertcnt
Set, but never read.

Caught by -Wunused-but-set-variable which is enabled in the newer
Fedora CI builders.
3 years ago
Jason Ish d2084ead84 github-ci: add Fedora 38 builders 3 years ago
Jason Ish f354dfc920 github-ci: add Fedora 37 builders 3 years ago
Jason Ish 5bd63541db github-ci: remove Fedora 35 build: EOL 3 years ago
Victor Julien 9c19757fe7 github-ci: remove debian 9 as it is EOL 3 years ago
Jason Ish aaa1688e03 github-ci: use bundle.sh script for libhtp, suricata-update
Update the GitHub CI workflow to use the bundle.sh script to pull in
Suricata-Update and libhtp. This means one less place where defaults
are hardcoded and can get out of sync.

This also simplifies the variable names that can be embedded in a pull
request message to use the same variable names that bundle.sh
expects. Of note, this removes the _PR variant, instead a branch name
of "pr/N" can be used to specify a PR.
3 years ago
Jason Ish 2993eea428 bundle.sh: allow a PR # to be specified
Allow pull requests (and merge requests) to be specified by using a
branch name like "pr/111" or "mr/222". This allows CI to use this
script as well, instead of multiple variations of the same thing.

Additonally allow the destination directory to be overridden with the
DESTDIR environment variable.
3 years ago
Justin Azoff a179f79eb6 detect/iponly: remove DetectEngineIPOnlyThreadCtx
This is unused.

Issue: 4578
(cherry picked from commit aacb7dc291)
3 years ago
Justin Azoff 03e767536a detect/iponly: Reduce the size of the SigNumArray bitsets
Instead of tracking ip only rules by the internal signum, track them by
a separate counter that starts at zero.  This results in dense
SigNumArrays instead of sparse ones and a much smaller max_idx.

Issue: 4578
(cherry picked from commit dfbc3da0eb)
3 years ago
Victor Julien f2cf58eec7 detect/content: add negated endswith test
(cherry picked from commit a42c225117)
3 years ago
Jeff Lucovsky 0a9b7d0528 detect/content: Negated endswith matches
Issue: 5541

This commit handles negated endswith matches.

(cherry picked from commit c083cbda33)
3 years ago
Victor Julien bb9880bad8 version: require libhtp 0.5.43 3 years ago
Victor Julien e0bb867d21 version: start development towards 6.0.12 3 years ago
Shivani Bhardwaj 144ea08e0e release: 6.0.11; update changelog 3 years ago
Jeff Lucovsky 7dc4bf740a doc/byte_math: Add divide by 0 discussion.
Issue: 5945
(cherry picked from commit fd46c93a8f)
3 years ago
Jeff Lucovsky a7fbfd572c detect/byte_math: fix bug in byte_math detection
Issue: 5945

Avoid division by zero when the byte_math operation is division and the
rvalue is 0.

(cherry picked from commit 38c5e89e29)
3 years ago
Shivani Bhardwaj 92d72344fb smtp: enforce line limit even when LF is found
Before:
If LF character was found, so far, we won't enforce the line limit on
the line. We only enforced limits in case of LF character missing in a
long line.

After this patch:
Line limit is enforced on the line if it is bigger than 4096 Bytes
irrespective of whether LF was found or not.

Redmine Bug: 5819

(cherry picked from commit 5f52b199ff)
3 years ago
Shivani Bhardwaj 08fd66718a smtp: reset current line len at start 3 years ago
Shivani Bhardwaj 564c7793a5 util/mime: allow delim len 0 when line limit is hit
(cherry picked from commit fd4e0fbafe)
3 years ago
Shivani Bhardwaj 707dc705d9 smtp: move constant declaration to header
(cherry picked from commit c0bff5f921)
3 years ago
Victor Julien 80abc3121d pcap: improve pcap_breakloop support
When pcap_breakloop has been issued on a handle, the current pcap_dispatch
call may return -2 (PCAP_ERROR_BREAK), but it can also return the number
of processed packets if lower than the desired number. So add this condition
as a check.

(cherry picked from commit 9fe08f2374)
(cherry picked from commit 2ddd26446e)
3 years ago
Victor Julien 6d8b50b748 detect: only breakloop threads that are lagging
Sleep after all threads have been checked.

Bug: #5969.
(cherry picked from commit 8a968faa04)
3 years ago
Victor Julien 6658300c84 flow/worker: refresh detect thread during housekeeping
During housekeeping multiple flows are processed. If a rule reload happens
at that time, we need to use the new detect thread as soon as possible.

Bug: #5969.
(cherry picked from commit 5e4cf182ab)
3 years ago
Philippe Antoine 42786aef59 http2: faster when reducing dynamic headers size
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)
3 years ago
Philippe Antoine 749277d46f http: complete multipart until request.body-limit
In the case we are truncating a multipart file because of reaching
request.body-limit, we used to not consume the whole buffer, but
keep expected_boundary_len bytes in case a new boundary begins
in these bytes.
Even if we cannot check the complete boundary, we can still check
the first bytes, as will be done in the rust version.

Ticket: #5952
(cherry picked from commit 578f328e06)
(cherry picked from commit caf9940fd1)
3 years ago
Victor Julien 2c600336ab stream: improve FIN checking
After recent next_seq changes, the FIN checks could be too strict
leading to stalling sessions in IPS mode.

This patch requires a FIN to be >= last ack and <= next_win to be
accepted.

(cherry picked from commit 39a6f411e9)
3 years ago
Shivani Bhardwaj 3442b6da9d util/base64: don't reset decoded bytes in RFC4648
Old behavior:
With RFC4648, the decoded bytes were reset to 0 in case an unusual
character was encountered in the encoded string. This worked out fine
for small test cases where there weren't many bytes to be decoded.

Problem:
If a big encoded string had a character outside of the base alphabet,
the processing would stop and the number of decoded bytes were set to 0.
However, even though the processing should stop at the invalid
character, the number of decoded bytes should correctly store the bytes
decoded up until the point an invalid characted was encountered.

New behavor:
For any base64 encoded string given to the base64 decoder in RFC4648
mode, we make sure that the number of decoded bytes correctly reflect
the number of bytes processed up until the string was valid. This makes
sure any further calculations/use of the decoded data is done correctly.

Redmine ticket: 5885

(cherry picked from commit 418ddba38e)
4 years ago
Philippe Antoine 3851504638 enip: optimized tx iterator
As for SMTP, having a linked list.

Ticket: #5927
(cherry picked from commit 4f7426fdcf)
4 years ago
Philippe Antoine ff2e02844b dnp3: optimized tx iterator
As for SMTP, having a linked list.

Ticket: #5927
(cherry picked from commit e15daf6a4b)
4 years ago
Philippe Antoine 8fd0862f94 smtp: optimized tx iterator
To be more efficient with larger number of transactions.
As was done for FTP.

Ticket: #5927
(cherry picked from commit f5f215dae7)
4 years ago
Eric Leblond 9dfb3baf19 sip: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond ce12cb85d7 enip: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 0a78edc20f ntp: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 94a7072384 krb: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 7137c877e8 mqtt: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 6cb79d0721 ike: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond b1af87709f dns: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 84cbdbf21f snmp: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
4 years ago
Eric Leblond 4a3fdedc1f app-layer: add flag to skip detection on TX
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
4 years ago
Eric Leblond 5c1995a78f detect: remove STREAM_FLUSH
It is unused in the code so can be removed.

Ticket: #5799
4 years ago