Jeff Lucovsky [Thu, 11 Jun 2020 12:31:55 +0000 (08:31 -0400)]
detect/transform: Add transform "validate" function
This commit adds an (optional) entry for a validation function. The
validation function, if present, will be used during rule processing.
Its role is to determine if the arguments are compatible with the
transform. E.g., a content string of "this string has whitespace" is not
compatible with the `strip_whitespace` transform.
Jason Ish [Fri, 26 Jun 2020 17:45:38 +0000 (11:45 -0600)]
file-hash-common: fix rule_file truncation
Loading file hash lists uses dirname(3) on the
de_ctx->rule_file which modifies the contents,
removing the last part of the path. So on subsequent
calls the rule_file no longer contains the rule_file,
but instead just the directory name.
Mostly noticed when using "-S" with rule files outside
of the default-rule-path which requires more hunting for
the rule file.
Angelo Mirabella [Tue, 16 Jun 2020 09:04:06 +0000 (10:04 +0100)]
detect/http_raw_header: Correct type mismatch
This changeset fixes a bug on the computation of the buffer
lenght for raw http headers. The bug is due to a mismatch
on the data type of the length (uint8_t vs uint32_t) and it
was causing signature misses.
Victor Julien [Sun, 5 Apr 2020 12:35:29 +0000 (14:35 +0200)]
decode: cleanup packet properly on bad packets
In case of bad IPv4, TCP or UDP, the per packet ip4vars/tcpvars/udpvar
structures would not be cleaned up because the cleanup depends on the
'header' pointer being set, but the error handling would unset that.
This could mean these structures were already filled with values before
the error was detected. As packets were recycled, the next packet decoding
would use this unclean structure.
To make things worse these structures are part of unions. IPv4/IPv6 and
TCP/ICMPv4/ICMPv6 share the same memory location.
LibFuzzer+UBSAN found this both locally and in Oss-Fuzz:
decode-ipv6.c:654:9: runtime error: load of value 6, which is not a valid value for type 'bool'
#0 0x6146f0 in DecodeIPV6 /src/suricata/src/decode-ipv6.c:654:9
#1 0x617e96 in DecodeNull /src/suricata/src/decode-null.c:70:13
#2 0x9dd8a4 in DecodePcapFile /src/suricata/src/source-pcap-file.c:412:9
#3 0x4c8ed2 in LLVMFuzzerTestOneInput /src/suricata/src/tests/fuzz/fuzz_sigpcap.c:158:25
#4 0x457e51 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
#5 0x457575 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:470:3
#6 0x459917 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:698:19
#7 0x45a6a5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:830:5
#8 0x448728 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:824:6
#9 0x472552 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
#10 0x7ff0d097b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#11 0x41bde8 in _start (/out/fuzz_sigpcap+0x41bde8)
Victor Julien [Fri, 3 Apr 2020 15:03:47 +0000 (17:03 +0200)]
ssl: fix handshake cert buffer sizing
'trec' buffer was not grown properly when it was checked as too small.
After this it wasn't checked again so that copying into the buffer could
overflow it.
Jeff Lucovsky [Fri, 24 Apr 2020 14:28:54 +0000 (10:28 -0400)]
napatech: Correct timestamp rounding issue
This commit fixes the conversion of timestamps. Without the extra
parens, the resulting timestamp value for usecs will be 1 or 0 due to
the operator precedence order (+ takes precedence over ?:)
Eric Leblond [Fri, 7 Feb 2020 23:05:01 +0000 (00:05 +0100)]
app-layer-expectation: clean expectation and add limits
When a flow timeout, we can have still existing expectations that
are linked to this flow. Given that there is a delay between the
real ending of the flow and its destruction by Suricata, the
expectation should be already honored so we can assume the risk
to clean the expectations that have been triggered by the
to-be-deleted flow.
This patch introduces a limitation in term of number of
expectations attached to one IPPair. This is done using
a circle list so we have a FIFO approach on expectation
handling.
Circleq list code is copied from BSD code like was pre existing code
in queue.h.
Philippe Antoine [Mon, 16 Mar 2020 13:48:40 +0000 (14:48 +0100)]
ftp: FTPGetAlstateProgress for done port commands
For a done transaction with command PORT,
we expect FTP_STATE_FINISHED
and we got FTP_STATE_PORT_DONE instead
which prevented logging of these transactions
We change the order of the evaluations to get the right result
Jason Ish [Thu, 9 Apr 2020 21:59:23 +0000 (15:59 -0600)]
conf/yaml: limit recursion depth while paring YAML
A deeply nested YAML file can cause a stack-overflow while
reading in the configuration to do the recursive parser. Limit
the recursion level to something sane (128) to prevent this
from happening.
The default Suricata configuration has a recursion level of 128
so there is still lots of room to grow (not that we should).
Victor Julien [Fri, 28 Feb 2020 12:17:03 +0000 (13:17 +0100)]
pcap/file: improve time handling
This patch addresses two problems.
First, various parts of the engine, but most notably the flow manager (FM),
use a minimum of the time notion of the packet threads. This did not
however, take into account the scenario where one or more of these
threads would be inactive for prolonged times. This could lead to the
time used by the FM could get stale.
This is addressed by keeping track of the last time the per thread packet
timestamp was updated, and only considering it for the 'minimum' when it
is reasonably current.
Second, there was a minor race condition at start up, where the FM would
already inspect the hash table(s) while the packet threads weren't active
yet. Since FM gets the time from the packet threads, it would use a bogus
time of 0.
This is addressed by adding a wait loop to the start of the FM that waits
for 'time' to get ready.
Victor Julien [Thu, 27 Feb 2020 16:20:18 +0000 (17:20 +0100)]
pcap/file: fix race during pcap processing start
A race condition during the start of pcap file processing could cause
missed alerts and logged events. This race happens between the packet
threads and the flow manager. It was observed on slower hardware, but in
theory could happen on any machine. It required the 'autofp' runmode.
In commit 6f560144c1b9 ("time: improve offline time handling") the logic
was added to make the flow manager use a minimum of all the packet threads
perception of time.
The race condition was that the flow manager may become active _before_
all of the packet threads have started processing packets and thus setting
their timestamp. The threads that had not yet initialized their timestamp
would not be considered when calculating the minimum.
As a result of this, older packets timestamps would not yet be registered.
This would give the Flow Manager a timestamp too far in the future. While
the FM was running, the packet processing would start and a flow would
be created. This flow would then immediately be considered 'timed out' by
the FM, due to the timestamp too far in the future.
In the observed case, the thread processing packet 1 from the pcap had not
yet started processing while other threads had already started. The FM was
also already active. Due to the timestamps in the pcap this meant that the
time the FM used was about 500 seconds in the future compared to packet 1.
This patch fixes the issue by initializing all of the threads timestamps
with the timestamp value of the first packet. This way the minimum will
always consider this timestamp.