Commit Graph

19178 Commits (28b10fb0429f42b1f1cfdb3bcb4ea634e3cbaab3)
 

Author SHA1 Message Date
Abhijeet Singh 28b10fb042 util/log: rotate log file periodically
Fix log file not rotating during zero traffic periods by triggering rotation logic every second

Ticket: https://redmine.openinfosecfoundation.org/issues/8115
1 week ago
Victor Julien 1eff94f2b2 github-ci: add gcc analyzer build
Make sure to not run against lua rust crate build, as it's not clean.
1 week ago
Victor Julien 7cb64cc20f nfq: suppress gcc analyzer warnings 1 week ago
Victor Julien 79ef58be12 mpm/hs: remove useless pointer check
Pointer can't be NULL, so don't check it.

Helps gcc analyzer as well.
1 week ago
Victor Julien f434732c73 util/var-name: help gcc analyzer
Help it understand TAILQ.
1 week ago
Victor Julien 32fc437568 spm/bm: match suff array to pattern size
Avoids gcc -fanalyzer warning about out of bounds write to the array.
1 week ago
Victor Julien 5179af5557 affinity: gcc analyzer warnings
The double strchr confused gcc -fanalyzer.
1 week ago
Victor Julien f9cf9c1de7 output: suppress gcc analyzer warnings
By teaching about TAILQ.
1 week ago
Victor Julien e2f9211727 log-pcap: address gcc analyzer warnings
Reopen file descriptor for lz4 with the init function. This helps
code analyzers understand the handle it's leaked.

Improve flow of profiling dumps to avoid analyzer confusion around the
file descriptor.

Suppress TAILQ related warnings.
1 week ago
Victor Julien 65f1a022f0 detect/ip_proto: clean up parsing function
Helps address a gcc -fanalyzer warning.
1 week ago
Victor Julien ff7fae54d2 detect/flowvar: help gcc -fanalyzer
Add debug validation statement to assert prev pointer is not NULL.
1 week ago
Victor Julien 05eac977f0 detect/sigorder: handle allocation failure
Addresses a gcc -fanalyzer warning.
1 week ago
Victor Julien caa9ddaa26 decode/tcp: only set data ptr for valid option lengths 1 week ago
Victor Julien 32414573a7 conf: assist gcc -fanalyzer
Work around TAILQ false positive.
1 week ago
Victor Julien 6e1f96f91a tm/queues: assist gcc -fanalyzer
Work around TAILQ false positive.
1 week ago
Victor Julien a5effad936 frames: avoid possible undefined behavior
Code analyzer flagged FrameCopy as a possible source of UB due to
both pointers passed to memcpy being the same.

app-layer-frames.c: In function ‘FrameCopy’:
app-layer-frames.c:236:5: warning: overlapping buffers passed as arguments to ‘memcpy’ [-Wanalyzer-overlapping-buffers]
  236 |     memcpy(dst, src, sizeof(*dst));
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘FramePrune’: events 1-8
    │
    │  750 | static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof)
    │      |             ^~~~~~~~~~
    │      |             |
    │      |             (1) entry to ‘FramePrune’
    │......
    │  766 |     for (uint16_t i = 0; i < frames->cnt; i++) {
    │      |                          ~~~~~~~~~~~~~~~
    │      |                            |
    │      |                            (2) following ‘true’ branch... ─>─┐
    │      |                                                              │
    │      |                                                              │
    │      |┌─────────────────────────────────────────────────────────────┘
    │  767 |│        if (i < FRAMES_STATIC_CNT) {
    │      |│           ~
    │      |│           |
    │      |└──────────>(3) ...to here
    │      |            (4) following ‘true’ branch (when ‘i <= 2’)... ─>─┐
    │      |                                                              │
    │      |                                                              │
    │      |┌─────────────────────────────────────────────────────────────┘
    │  768 |│            Frame *frame = &frames->sframes[i];
    │      |│                            ~~~~~~~~~~~~~~~~~~
    │      |│                                           |
    │      |└──────────────────────────────────────────>(5) ...to here
    │  769 |             FrameDebug("prune(s)", frames, frame);
    │  770 |             if (eof || FrameIsDone(frame, acked)) {
    │      |                ~
    │      |                |
    │      |                (6) following ‘false’ branch... ─>─┐
    │      |                                                   │
    │......
    │      |                                                   │
    │      |┌──────────────────────────────────────────────────┘
    │  779 |│                const uint64_t fle = FrameLeftEdge(stream, frame);
    │      |│                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │      |│                                     |
    │      |└────────────────────────────────────>(7) ...to here
    │      |                                      (8) calling ‘FrameLeftEdge’ from ‘FramePrune’
    │
    └──> ‘FrameLeftEdge’: event 9
           │
           │  257 | static inline uint64_t FrameLeftEdge(const TcpStream *stream, const Frame *frame)
           │      |                        ^~~~~~~~~~~~~
           │      |                        |
           │      |                        (9) entry to ‘FrameLeftEdge’
           │
         ‘FrameLeftEdge’: event 10
           │
           │suricata-common.h:323:27:
           │  323 |         #define BUG_ON(x) assert(!(x))
           │      |                           ^~~~~~
           │      |                           |
           │      |                           (10) following ‘false’ branch (when ‘frame_offset <= app_progress’)... ─>─┐
           │      |                                                                                                     │
util-validate.h:95:36: note: in expansion of macro ‘BUG_ON’
           │   95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp))
           │      |                                    ^~~~~~
app-layer-frames.c:266:5: note: in expansion of macro ‘DEBUG_VALIDATE_BUG_ON’
           │  266 |     DEBUG_VALIDATE_BUG_ON(frame_offset > app_progress);
           │      |     ^~~~~~~~~~~~~~~~~~~~~
           │
         ‘FrameLeftEdge’: event 11
           │
           │      |                                                                                                     │
           │      |┌────────────────────────────────────────────────────────────────────────────────────────────────────┘
           │  269 |│    if (frame->len < 0) {
           │      |│        ~~~~~^~~~~
           │      |│             |
           │      |└────────────>(11) ...to here
           │
    <──────┘
    │
  ‘FramePrune’: events 12-13
    │
    │  779 |                 const uint64_t fle = FrameLeftEdge(stream, frame);
    │      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │      |                                      |
    │      |                                      (12) returning to ‘FramePrune’ from ‘FrameLeftEdge’
    │......
    │  783 |                 FrameCopy(nframe, frame);
    │      |                 ~~~~~~~~~~~~~~~~~~~~~~~~
    │      |                 |
    │      |                 (13) calling ‘FrameCopy’ from ‘FramePrune’
    │
    └──> ‘FrameCopy’: events 14-15
           │
           │  234 | static void FrameCopy(Frame *dst, Frame *src)
           │      |             ^~~~~~~~~
           │      |             |
           │      |             (14) entry to ‘FrameCopy’
           │  235 | {
           │  236 |     memcpy(dst, src, sizeof(*dst));
           │      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           │      |     |
           │      |     (15) ⚠️  overlapping buffers passed as arguments to ‘memcpy’
           │
In file included from suricata-common.h:129,
                 from app-layer-frames.c:25:
/usr/include/string.h:47:14: note: the behavior of ‘memcpy’ is undefined for overlapping buffers
   47 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
      |              ^~~~~~
1 week ago
Victor Julien 188376b577 output/filestore: refactor file descriptor handling
To assist code analyzers. Gcc -fanalyzer got confused about it.

Also test data pointer and length before calling fwrite and check the
result better.

Use a single atomic for the max open files check.
1 week ago
Philippe Antoine c6014a77a2 websocket: accepts config value with units
Ticket: 8552

As was the commented out example
1 week ago
Samaresh Kumar Singh 565e138754 pcap-file: skip setvbuf on non-seekable streams
Reading a pcap from /dev/stdin or a named pipe currently fails with "failed to get first packet timestamp. pcap_next_ex(): -1" because InitPcapFile calls setvbuf on the FILE* underlying the pcap handle after libpcap has already consumed the pcap header. On a non-seekable fd glibc cannot recover from that and the next read returns -1.

Detect non-regular files via fstat and skip setvbuf in that case so the read keeps working on pipes, fifos and stdin.

Accept pcap-file.buffer-size values of 0, which disables setvbuf buffering as an explicit opt-out, or PCAP_FILE_BUFFER_SIZE_MIN (4 KiB) to PCAP_FILE_BUFFER_SIZE_MAX (64 MiB). Treat any non-zero setvbuf return value as an error, not just negative values.

When pcap-file.buffer-size fails to parse, retain the default buffer size instead of falling through and setting it to 0. The branches are now mutually exclusive so only one of the parse-error, accepted, or out-of-range messages is logged.

Update the user guide: --pcap-file-buffer-size now documents valid values of 0 (disables setvbuf buffering) or 4 KiB to 64 MiB, and pcap-file.rst notes that 0 is the opt-out for non-seekable sources such as stdin and named pipes.
Bug: #8464.
1 week ago
Samaresh Kumar Singh df353242bd detect: fail thread init on keyword ctx error
DetectEngineThreadCtxInitKeywords returns TM_ECODE_FAILED when a per-thread keyword init fails (for example DetectFilemagicThreadInit), but ThreadCtxDoInit discarded that result and still returned OK. The detect thread then ran with a partially initialized keyword context array, producing indeterminate results. Propagate the failure so the callers abort thread init and clean up.

Add a unit test that registers a keyword whose thread init fails and verifies that DetectEngineThreadCtxInit reports the failure.

Ticket: #8237
1 week ago
Philippe Antoine 6bb271cee9 conf: uses SCConfGetNonNull
Ticke: 8651

Uses it in place when we dereferenced the value straight away
after checking SCConfGet result but not its value
1 week ago
Philippe Antoine a9e1dff4a6 conf: introduce SCConfGetNonNull
Ticket: 8651

Behaves like SCConfGet but returns 0 on null value
1 week ago
Philippe Antoine e0152178da rust: format detect files
Ticket: 3836
1 week ago
Giuseppe Longo 8a4eea44d8 suricata.yaml.in: enable app-layer.sip by default 2 weeks ago
Giuseppe Longo ae58cc6491 rules: add sip-events.rules
Ticket #8524
2 weeks ago
Giuseppe Longo 695b4f4a3d sip: remove incomplete data event
It's no longer useful to set an event when the data is incomplete,
since an error is returned.

Ticket #8524
2 weeks ago
Giuseppe Longo ba5e850264 sip: make pattern matching more robust
SIP and SSDP share method names like NOTIFY and SUBSCRIBE,
causing SSDP traffic to be misidentified as SIP.
Add a probing parser callback that checks for "SIP/" in the
payload before accepting a pattern match.

Example of a misidentified flow before the fix:
{"timestamp":"2014-02-27T19:44:43.164211+0100","flow_id":986757542077835,"event_type":"flow","src_ip":"192.168.1.1","src_port":9489,"dest_ip":"239.255.255.250
","dest_port":1900,"ip_v":4,"proto":"UDP","app_proto":"sip","flow":{"..."}}

After the fix:
{"timestamp":"2014-02-27T19:44:43.164211+0100","flow_id":986757542077835,"event_type":"flow","src_ip":"192.168.1.1","src_port":9489,"dest_ip":"239.255.255.250
","dest_port":1900,"ip_v":4,"proto":"UDP","app_proto":"failed","flow":{"..."}}

Ticket #8355
2 weeks ago
Philippe Antoine 762b725e1f ci: check-doc-rules copies data(sets) files into tmpdir
Allows to use datasets load
2 weeks ago
Victor Julien d154484cc6 detect/firewall: support hook LTE mode for built-in hooks
LTE support depended on registered hook names, but did not support the
built-in names. This commit adds the support.

Ticket: #8645.
2 weeks ago
Victor Julien b02fa53134 detect: add helper for getting hook name
To fetch hook name from app-layer, or use built-ins.
2 weeks ago
Victor Julien 2a2cb40336 detect: misc debug log additions 2 weeks ago
Victor Julien 569d27e153 detect: propagate inspect engine setup failures 2 weeks ago
Victor Julien d64954a873 detect: don't register unrelated inspect engines
For rules that specify an explicit app-layer hook,
e.g. http1:request_headers, don't register inspect engines for
other protocols like HTTP/2. These have their own progress tracking,
so should be excluded from these rules.
2 weeks ago
Urval 3b9dc5ad37 output-json: avoid freeing caller-owned JSON builder
EveEmailLogJsonData() freed the provided SCJsonBuilder when
SMTP state was unavailable, even though ownership remains with
the caller. The caller may continue using the builder for
cleanup after the function returns failure.

Return failure without freeing the builder and add defensive
NULL checks in EveSmtpDataLogger().

Signed-off-by: Urval Kheni <urvalkheni777@gmail.com>
2 weeks ago
Shivani Bhardwaj 0345b91ddd rust: exclude DETECT_BYTEMATH_ENDIAN_DEFAULT from bindings
to deal with the failure due to cbindgen updates and mismatches in
generated bindings.

detect-bytemath.c:61: error: "DETECT_BYTEMATH_ENDIAN_DEFAULT" redefined [-Werror]
   61 | #define DETECT_BYTEMATH_ENDIAN_DEFAULT (uint8_t) BigEndian
      |
In file included from rust.h:34,
                 from detect-bytemath.c:32:
./../rust/gen/rust-bindings.h:5071: note: this is the location of the previous definition
 5071 | #define DETECT_BYTEMATH_ENDIAN_DEFAULT BigEndian
      |
2 weeks ago
Denis Balashov 3b5bdc4261 examples: check rate filter callback registration result
Check DetectEngineEnabled() before registering, and warn if the
registration call returns false.
2 weeks ago
Denis Balashov 17151571b8 detect: guard rate filter callback registration, return bool on failure
SCDetectEngineRegisterRateFilterCallback() dereferences the result of
DetectEngineGetCurrent() unconditionally. Add a NULL guard with
SCLogError and return false so callers can detect registration failure.

Flagged by Svace and confirmed by gcc -fanalyzer.

Ticket: 8560
2 weeks ago
Denis Balashov 4d4c6227e1 detect/parse: assert HashListTableLookup results in duplicate sig check
Add DEBUG_VALIDATE_BUG_ON() at four sites in DetectEngineSignatureIsDuplicate()
where HashListTableLookup() is assumed to return non-NULL. Documents the
invariant that every Signature in sig_list has a dup_sig_hash_table entry
and catches violations in debug builds.

Flagged by Svace static analyzer.

Ticket: 8635
2 weeks ago
Denis Balashov 3828de20ed decode: propagate PacketAlertCreate failure instead of crashing
PacketAlertCreate is called from PacketInit on the packet allocation
path. Make PacketInit return bool and propagate the NULL result from
PacketAlertCreate up through PacketGetFromAlloc, which already returns
NULL to signal allocation failure to its callers.

Update the UNITTESTS-only helpers in defrag.c accordingly: helpers
returning Packet * use an explicit NULL check; the one returning int
keeps the existing FAIL_IF style.
2 weeks ago
Denis Balashov 344e89e3c6 qa/cocci: fix broken regex alternation in malloc-error-check
Coccinelle uses OCaml Str, not PCRE. The '|' and '()' characters are
literals in OCaml Str, so 'identifier func =~ "(SCMalloc|SCCalloc|...)"'
never matched anything — making the entire script a no-op since its
introduction.

Replace all five patterns with OCaml Str alternation syntax 'A\|B'.

Ticket: 8641
2 weeks ago
Denis Balashov 7124fdebf1 tests/fuzz: guard SCCalloc result in fuzz_decodebase64
If SCCalloc fails, decoded is NULL and the subsequent SCBase64Decode
call would dereference it. Return early on allocation failure.
2 weeks ago
Denis Balashov 2c16fb716b util/mpm-hs: fix null check parentheses; simplify SCHSConfigInit
Two fixes:
- Remove extra parentheses in existing NULL check: (*ext) -> *ext,
  which was causing the cocci script to miss the check as a false negative.
- Simplify SCHSConfigInit to return SCCalloc() directly; the caller
  in detect-engine.c already checks the return value for NULL.
2 weeks ago
Denis Balashov 04a1de1192 detect/reference: guard SCStrdup calls in DetectReferenceParse
Two SCStrdup calls that set ref->key had no NULL check. On allocation
failure the pointer would be used immediately, causing a NULL dereference.
2 weeks ago
Denis Balashov 01f64ea335 detect/flowbits: check SCRealloc result before overwriting pointer
The original pointer was overwritten with the SCRealloc result before
checking for NULL, causing a memory leak if reallocation fails.
Check the temporary pointer first before assigning.
2 weeks ago
Denis Balashov 21bcbe14c1 detect/alert: guard SCStrdup result before use
SCStrdup result was stored and immediately used without checking for
NULL, which would cause a NULL dereference if allocation fails.
2 weeks ago
Denis Balashov b2cd08bb91 util/log-redis: guard SCCalloc result for redis stream format
When Redis output is configured in stream/xadd mode with a positive
stream-maxlen, SCConfLogOpenRedis() allocates redis_setup.stream_format
and immediately passes it to snprintf().

If SCCalloc() fails, snprintf() receives a NULL destination pointer and
the process can crash during Redis output initialization. Handle this
unrecoverable setup failure with FatalError(), matching the surrounding
Redis initialization error handling.

Ticket: 8588
2 weeks ago
dependabot[bot] 39688a6252 github-actions: bump codecov/codecov-action from 6.0.1 to 7.0.0
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 6.0.1 to 7.0.0.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](e79a6962e0...fb8b3582c8)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-version: 7.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
2 weeks ago
dependabot[bot] 6150d61d79 github-actions: bump github/codeql-action from 4.36.1 to 4.36.2
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.36.1 to 4.36.2.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Commits](https://github.com/github/codeql-action/compare/v4.36.1...v4.36.2)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: 4.36.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
2 weeks ago
Giuseppe Longo d892eb8e0e doc/sctp: add sctp keywords
Add documentation for all sctp keywords.

Ticket #4251
2 weeks ago
Giuseppe Longo dc44447ce6 detect/sctp: add sctp.chunk_data sticky buffer
Add a sctp.data sticky multi-buffer that allows content matching on the bytes
inside any of the SCTP DATA chunks extracted.

Ticket #4251
2 weeks ago