Jason Ish [Mon, 30 Jun 2025 21:55:21 +0000 (15:55 -0600)]
lib: opt-in signal handlers
Instead of enabling signal handlers by default, require the user of
the library to opt-in. This is done with the call to
SCEnableDefaultSignalHandlers, which sets a flag to add the default
signal handlers.
This seems like the least invasive way to do this at this time, but it
will require some re-thinking for 9.0, especially if migrate globals
to engine instances, signal handling will need to be re-thought.
Configuration option `threads: auto` in DPDK's interface node
overassigns available threads to the interface.
Commit 4dfd44d3 changed the signedness of the remaining threads counter,
which caused surpass of the counter initialization.
The if-clause is switched to first initialize and then use the counter.
Jason Ish [Mon, 30 Jun 2025 14:56:55 +0000 (08:56 -0600)]
rust/htp: follow suricata versioning
Have htp follow Suricata versioning so we don't have to worry about
version updates as it changes.
For example, between 8.0.0-beta1 and 8.0.0-rc1 there were changes to
the htp, however the version stayed at 2.0.0 making it impossible to
publish these changes to crates.io.
Boris Tonofa [Mon, 30 Jun 2025 13:39:47 +0000 (16:39 +0300)]
detect-bytetest: remove meaningless NULL check on data_offset
The condition data_offset == NULL can never be true: data_offset has
already been validated as non-NULL a few lines earlier. The guard seems
to have been intended for the offset argument, yet throughout the
codebase offset is never passed as NULL. (In the unit tests, offset is
NULL, but those tests pass the value parameter as NULL, which causes the
function to return before offset is dereferenced.)
Remove the pointless check to simplify control flow and silence
static-analysis warnings.
Shivani Bhardwaj [Mon, 30 Jun 2025 10:24:35 +0000 (15:54 +0530)]
smtp: trigger raw stream inspection
Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.
Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.
Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.
Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.
SMTP parser can handle multiple command lines per direction. Appropriate calls
to trigger raw stream inspection have been added on succesful parsing of each
request line and response line.
For the requests, the call to trigger inspection has been added in the
beginning rather than the completion of transactions. This does not
affect the inspection as it is actually triggered in the following call.
This covers the case for anomaly as well. There are two benefits for
this:
- immediate inspection for anomalous data
- flushing of the anomalous data making next data's inspection cleaner
to build the correct behavior. As a part of ab01a1b, in order to match
the behavior in master, the calls for triggering raw stream inspection
were made when communication in one direction for a transaction was
completed. However, it was incorrect to do so. Reliable inspection
requires any request line/response line to be completed.
Fupeng Zhao [Thu, 19 Jun 2025 06:03:50 +0000 (14:03 +0800)]
util/coredump: refactor parsing and respect zero core dump limit
- Replaced strtoul/strtoull with ByteExtractString* for safer and more consistent parsing.
- Allowed max-dump to be set to 0, and correctly apply a core dump limit of 0, maintaining behavior consistent with the commented default in suricata.yaml.in.
- Added and registered unit tests to validate the updated logic.
Lukas Sismis [Wed, 25 Jun 2025 09:18:56 +0000 (11:18 +0200)]
affinity: initialize CPU sets with online CPUs only
When no CPU set is explicitly defined, switch from
UtilCpuGetNumProcessorsConfigured() (which counts all existing CPU
cores, even offline ones) to UtilCpuGetNumProcessorsOnline() (only
the available cores).
If Suricata initializes more threads than online CPUs it oversubscribes
the system. As Suricata does not support any runtime live reconfiguration
Suricata initializes only as many cores as online CPU cores.
Victor Julien [Sat, 21 Jun 2025 19:13:35 +0000 (21:13 +0200)]
nfq: suppress coverity thread warning
CID 1593187: (#1 of 1): Data race condition (MISSING_LOCK)
2. missing_lock: Accessing (*p).nfq_v.mark without holding lock Packet_.persistent.tunnel_lock. Elsewhere, NFQPacketVars_.mark is written to with Packet_.persistent.tunnel_lock held 2 out of 5 times (2 of these accesses strongly imply that it is necessary).
No concurrency happening on non-tunnel packet, so no locking needed.
CID 1554228: (#1 of 1): Indefinite wait (BAD_CHECK_OF_WAIT_COND)
dead_wait: A wait is performed without ensuring that the condition is not already satisfied while holding lock PktPoolLockedStack_.mutex. This can cause a deadlock if the notification happens before the lock is acquired.
Acquire the lock, then check the wait condition in a loop, without releasing with the lock before the wait. This will prevent deadlocks and failed conditions from spurious wakeups.
CID 1554214: (#1 of 1): Indefinite wait (BAD_CHECK_OF_WAIT_COND)
dead_wait: A wait is performed without ensuring that the condition is not already satisfied while holding lock ThreadVars_.ctrl_mutex. This can cause a deadlock if the notification happens before the lock is acquired.
Acquire the lock, then check the wait condition in a loop, without releasing with the lock before the wait. This will prevent deadlocks and failed conditions from spurious wakeups.
In flow manager and recycler timed condition wait loops.
First check loop break conditions before entiring the timed wait.
CID 1638284: (#1 of 1): Indefinite wait (BAD_CHECK_OF_WAIT_COND)
dead_wait: A wait is performed without ensuring that the condition is not already satisfied while holding lock flow_manager_ctrl_mutex. This can cause a deadlock if the notification happens before the lock is acquired.
CID 1638293: (#1 of 1): Indefinite wait (BAD_CHECK_OF_WAIT_COND)
dead_wait: A wait is performed without ensuring that the condition is not already satisfied while holding lock flow_recycler_ctrl_mutex. This can cause a deadlock if the notification happens before the lock is acquired.
Victor Julien [Thu, 19 Jun 2025 10:52:32 +0000 (12:52 +0200)]
defrag: improve thread safety in config logging
CID 1554235: (#1 of 1): Data race condition (MISSING_LOCK)
missing_lock: Accessing defragtracker_spare_q.len without holding lock DefragTrackerStack_.m. Elsewhere, DefragTrackerStack_.len is written to with DefragTrackerStack_.m held 2 out of 2 times.
Victor Julien [Thu, 19 Jun 2025 10:33:16 +0000 (12:33 +0200)]
datasets: use locking wrappers everywhere
To assist coverity, which got confused:
CID 1649393: (#1 of 1): Data race condition (MISSING_LOCK)
missing_lock: Accessing sets without holding lock sets_lock. Elsewhere, sets is written to with sets_lock held 2 out of 3 times.
Jason Ish [Thu, 19 Jun 2025 17:57:41 +0000 (11:57 -0600)]
lua: don't accept a table as a return value from match
Remove the half finished support for accepting a table returned from a
Lua rule's match function. This is not documented, not tested, and not
really implemented.
Also, use lua_tointeger to get the return value from the match function
as an integer instead of a float.
Eric Leblond [Mon, 16 Jun 2025 09:23:43 +0000 (11:23 +0200)]
datasets: remove useless NULL check
Coverity did detect that the cleaning code is only reached with
Dataset *set being initialized so the check is useless.
** CID 1649392: Null pointer dereferences (REVERSE_INULL)
/src/datasets-context-json.c: 719 in DatajsonGet()
>>> Null-checking "set" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
** CID 1649391: Null pointer dereferences (REVERSE_INULL)
/src/datasets.c: 526 in DatasetGet()
>>> Null-checking "set" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
Jason Ish [Mon, 16 Jun 2025 22:34:36 +0000 (16:34 -0600)]
windows: use _tzname instead of tzname
tzname is a POSIX variable, WIN32 has prefixed many POSIX variables
with "_". While Mingw64 supports both, UCRT64 emits a compiler warning
on the usage of "tzname".
This triggered a rather large clang-format update.
Andreas Herz [Wed, 11 Jun 2025 08:47:45 +0000 (10:47 +0200)]
detect/dataset: skip adding localstatedir if fullpath is provided
When the option to set a full path is enabled and a full path is
provided, skip adding the prefix (based on localstatedir) to the
directory since it would be unexpected and unwanted by a user.
Eric Leblond [Wed, 11 Jun 2025 12:02:19 +0000 (14:02 +0200)]
datajson: limit impact of feature for non user
The det_ctx structure was inflated by the additoin of the array to
handle JSON context. This commit updates the code to use a growing
buffer and limit the impact.
Eric Leblond [Mon, 9 Jun 2025 09:30:26 +0000 (11:30 +0200)]
detect/dataset: check context_key validity
As context_key is an user entry and as it is used to build the JSON
string of alert, we could end up with an invalid event if the string
contains improper characters.