Commit Graph

19158 Commits (6bb271cee97304025e76fc9799d0107af5995dd2)
 

Author SHA1 Message Date
Victor Julien fbaaa9dcae github-actions: update scan-build to clang-22
Run it on Ubuntu 26.04.

Update enabled checkers.

Ticket: #3153.
4 months ago
Victor Julien 524503b572 detect/prefilter: rewrite loop to assist scan-build
Also address a theoretical edge case where memory allocation failure
could lead to overrunning the sids array.
4 months ago
Victor Julien e5650ebcbd scan-build: suppress more warnings 4 months ago
Victor Julien 02f6a4fd87 detect: work around scan-build warning
detect-engine.c:2350:69: warning: Out of bound access to memory after the end of 'new_det_ctx' [security.ArrayBound]
 2350 |             FlowWorkerReplaceDetectCtx(SC_ATOMIC_GET(s->slot_data), new_det_ctx[i]);
      |                                                                     ^~~~~~~~~~~~~~
1 warning generated.
4 months ago
Victor Julien dc6696ac04 bpf: work around scan-build warning
suricata.c:574:5: warning: Potential out of bound access to the heap area with tainted index [security.ArrayBound]
  574 |     bpf_filter[nm] = '\0';
      |     ^~~~~~~~~~~~~~
1 warning generated.
4 months ago
Victor Julien 06dcb5aeb0 detect/flowbits: fix scan-build 0-size alloc warning
detect-flowbits.c:761:29: warning: Call to 'calloc' has an allocation size of 0 bytes [optin.portability.UnixAPI]
  761 |                             SCCalloc(sids_array_size, sizeof(uint32_t));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./util-mem.h:36:18: note: expanded from macro 'SCCalloc'
   36 | #define SCCalloc calloc
      |                  ^
1 warning generated.
4 months ago
Victor Julien 0c9e17b454 detect/file.data: suppress scan-build false positive 4 months ago
Victor Julien af937f61dc defrag: rearrange Frag struct to save 8 bytes
Avoid large gaps in the layout.
4 months ago
Victor Julien 9d693a72b6 stream: minor code cleanups 4 months ago
Victor Julien 24e7f3439e stream: don't double check pointer
Checked right above it as well.
4 months ago
Victor Julien d17c0bf096 stream: remove error messages in the packet path
Would only happen during allocation failure.
4 months ago
Victor Julien 2bcebe5285 stream: avoid double memset
Already done by StreamTcpThreadCacheGetSegment().
4 months ago
Victor Julien 474788fd49 pool: remove InitData argument
Only used in unittests.

Update Init callback and update callers.
4 months ago
Victor Julien dc05422769 defrag: remove pool Init callback
Built-in callback does the same thing (memset).
4 months ago
Victor Julien a27b9a6c49 defrag: remove unused InitData argument 4 months ago
Victor Julien af6ec14688 pool/thread: remove unused InitData argument
Only used in unittests, not by production code.
4 months ago
Victor Julien d0ad6ddf19 pool: remove unused Free callback
Not used so not tested.
4 months ago
Victor Julien d4575f5765 pool/thread: remove unused Free callback
Not used so not tested.
4 months ago
Victor Julien 4624028722 pool: fix scan-build warnings
util-pool.c:60:28: warning: Subtraction of a non-null pointer (from variable 'data') and a null pointer (via field 'data_buffer') results in undefined behavior [core.NullPointerArithm]
   60 |     ptrdiff_t delta = data - p->data_buffer;
      |                       ~~~~ ^    ~~~~~~~~~~~
util-pool.c:257:13: warning: Argument to 'free()' is offset by 24 bytes from the start of memory allocated by 'calloc()' [unix.Malloc]
  257 |             SCFree(pb);
      |             ^~~~~~~~~~
./util-mem.h:38:16: note: expanded from macro 'SCFree'
   38 | #define SCFree free
      |                ^
util-pool.c:262:9: warning: Attempt to release already released memory [unix.Malloc]
  262 |         SCFree(p->pb_buffer);
      |         ^~~~~~~~~~~~~~~~~~~~
./util-mem.h:38:16: note: expanded from macro 'SCFree'
   38 | #define SCFree free
      |                ^
3 warnings generated.

Also, after fixing these, this one came up.

  CC       util-pool.o
util-pool.c:244:9: warning: Attempt to release already released memory [unix.Malloc]
  244 |         SCFree(p->data_buffer);
      |         ^~~~~~~~~~~~~~~~~~~~~~
./util-mem.h:38:16: note: expanded from macro 'SCFree'
   38 | #define SCFree free
      |                ^
1 warning generated.

Address this as well.
4 months ago
Philippe Antoine bf64b52b95 http2: better compression against decompression bombs
Ticket: 8513

Suricata decides at 2 levels if a http2 flow is doing a compression
bomb.

There is a direct computation when one chunk of TCP data
is being parsed.
In this case, do not take the ratio into account, just use the size
of the decompressed data, so that if we get a big chunk of TCP data
like 1 MiB, and a not so high ratio of 200, we do not trigger
the debug assertion in util-file.c about 64MiB

The other case stays unchanged : when accumulating over the lifetile of
a flow with multiple txs, take into account the compression ratio,
so that a flow of many txs, having a super high (brotli) compression
ratio, ends up classified as a compression bomb.
(For example, having 100 txs each turning a 100 byte input into a 700 KiB
one)
4 months ago
Philippe Antoine 9aaa6f7854 http1: do not re-parse Content-Disposition header
Ticket: 8529

When Suricata handles a HTTP1 response body, it does so with a
file, and tries to get the filename from the Content-Disposition
header if any, then from the uri.

If it failed to find a file name, it tried again every time
there was new data from the response body, even if there was
no new data to find a file name in either the header nor the uri.

This causes a slowdown in the case the Content-Disposition header
is big.

Fix is to set the flag on the first call of the callback, to be
sure that we will parse the Content-Disposition header for a
filename header only once per http1 response.
4 months ago
Philippe Antoine 6d437956e2 detect/transforms: dotprefix can be chained
Ticket: 8537

Otherwise, it may cause a use-after-free, in case of reallocated
buffer and we used the buffer inspect which was freed.
4 months ago
Philippe Antoine 2b20a436e7 detect/transforms: decompress checks earlier if it is in-place
Ticket: 8536

Otherwise, it may cause a use-after-free.

So, need to allocate the temporary vector, before calling
SCInspectionBufferCheckAndExpand
4 months ago
Philippe Antoine 4c42998feb detect/engine: helper to know if a transform happens in-place 4 months ago
Philippe Antoine 7bf48b02be http2: protection against decompression bombs
Ticket: 8513

During decompression, fail early if we have a big decompression
ratio, and enough data.
Track this data also during a tx lifetime, and even a flow/state
lifetime, so that we set event and fail also if the compression
bomb is split over multiple packets
4 months ago
Philippe Antoine 29e4b08647 http2: code refactoring to split big function 4 months ago
Jeff Lucovsky 1dc629d978 defrag: release tracker lock before decoding reassembled packet
Decoding the reassembled packet inside DefragInsertFrag while holding
the tracker mutex creates a deadlock situation if the reassembled packet
contains tunneled fragments: two threads could each hold one tracker
mutex and deadlock on the other. Move the decode in Defrag() after
DefragTrackerRelease() so no lock is held during decode (and possibly
re-entering the defrag system).

Make sure to fetch tracker->ip_hdr_offset after DefragInsertFrag since
it may be written during that call if the first fragment arrives last.

Issue: 8550
4 months ago
Philippe Antoine 7a9fc64882 detect/lua: move allow_restricted_functions out of struct
Ticket: 8556

Allows hardening against buffer overflos in the structure.
Reduces the scope of the variable.
4 months ago
Philippe Antoine 0b239c4848 lua: add bounds checks for flowints and flowvars
Ticket: 8556
4 months ago
Philippe Antoine e98d419d96 ldap: bound the number of responses
Ticket: 8405
4 months ago
Jason Ish 078c08d84b dnp3: bound response reassembly
Ticket: 8460
4 months ago
Philippe Antoine 38d41e6fa6 dnp3: fix off-by-one in array sizes
So that we can write a final zero without overflowing the array
and relying on padding to avoid further problems

Ticket: 8448
4 months ago
Philippe Antoine c25ccd299e dnp3: explicit application layer length is bounded in uint16_t
Ticket: 8460
4 months ago
Philippe Antoine 658684a252 dnp3: bounds reassembly
Ticket: 8460
4 months ago
Philippe Antoine c977b2f31b frames: make sure we use a lowercase buffer name
Ticket: 8526

As some C protocols accepted case-insensitive frame names,
a rule using the same frame twice with different cases,
ended up using 2 different buffers,
which ended up in triggering a buffer overflow
4 months ago
Victor Julien 6ebd0c3cc2 http2: check for HTTP1 protocol during upgrade path
Ticket: 8492
4 months ago
Jason Ish 2d6c3213ae lua: enforce allocation limit on first alloc
Instead of just on re-alloc.

Ticket: #8507
4 months ago
Philippe Antoine 24527d662a defrag: check tracker/packet family in compare
Ticket: 8510

Without this check, in case of a hash collision, and the other
fields are equal, we could end up consider adding an IPv6 packet
to a IPv4 tracker (or vice versa).

Doing so, we end up interpreting an IPv6 packet as an IPv4 one,
and we do not benefit from the bounds checks from ipv4/ipv6 decoder.
4 months ago
Jason Ish 74eb82a8a5 datasets: fix path setup when load and save used with different paths
If load and save were used in the same rule with different paths, proper path
setup was skipped.

Ticket: #8546
4 months ago
Philippe Antoine 10bde66586 nfs: bound file_additional_procs to 1
Ticket: 8418

As we only need to know we saw at least one NFSPROC3_COMMIT
4 months ago
Philippe Antoine aea7ee21b8 nfs: bound namemap by using lru
Ticket: 8418
4 months ago
Philippe Antoine a50f494ef6 nfs: bound requestmap and use lru
Ticket: 8418
4 months ago
Victor Julien 367ca7f430 detect/tx: minor debug additions and fixes 4 months ago
Victor Julien b29226c7ea firewall/analyzer: include all hooks
For protocols using default 0-1 states, add support.

For others, print 'unknown' if no name is yet supported.

Ticket: #8514.
4 months ago
Victor Julien f7c44c4c23 detect/analyzer: log actual policy for app firewall 4 months ago
Victor Julien 7134592fea detect/firewall: configurable default policies
Allow configurable policies, including accept. For app-layer this
requires looping all available hooks to apply the policies.

Support configurable policies for packet-filter, pre-stream, pre-flow.

If there are no rules there is also no rule group (sgh). Make sure
the app hooks policies are correctly handled in this case by allowing
a NULL sgh to be handled as well.

For tx rule match actually apply drop directly. Previously this was
always handled by the default drop:flow policy.

Ticket: #7701.
4 months ago
Victor Julien eaacb41aaf firewall: accept:flow no longer implies pass:flow
Previously a `accept:flow` action would act as both a firewall "accept" and
a threat detection "pass" for the rest of the flow.

This patch changes that. The `accept:flow` action now only accepts the
rest of the packets for the firewall ruleset, but does still continue
threat detection rule evaluation.

Ticket: #8444.
4 months ago
Victor Julien 0093bd6123 detect/firewall: rename flow control variable
For improved readability.
4 months ago
Victor Julien e76728a536 firewall: support multi-action statements in rules
For firewall rules, allow multiple actions to be specified in a list

        accept:flow,pass:flow,alert
        accept:flow,alert
        accept:flow,pass:flow

It is mandatory to make the first action the primary firewall policy
action: accept, drop, reject.

Ticket: #8480.
4 months ago
Victor Julien b7a625d368 detect: suppress noisy debug messages
Fixes: 232276ac19 ("detect: ethernet/arp matching")
4 months ago