Andreas Herz [Wed, 13 Dec 2017 23:59:30 +0000 (00:59 +0100)]
rule-reload: fix possible hangup with SIGUSR2
In some cases the rule reload could hang. The pending USR2 signals will
be recognized even with the <2 check. Also the SCLogWarning shouldn't be
used in the handler (see Warning about SCLog* API above in the code).
Victor Julien [Wed, 13 Dec 2017 09:28:19 +0000 (10:28 +0100)]
app-layer/counters: check counter id
Check counter id before updating a counter. In case of a disabled
parser with the protocol detection enable, the id can be 0. In
debug mode this would lead to a BUG_ON.
Command line option pcap-file-continuous was ignoring command line options
passed after its usage. Fixed bug, fixed formatting of help command
regarding pcap-file-continuous.
conf: multiple NULL-pointer dereferences in StreamTcpInitConfig
There are several NULL-pointer derefs in StreamTCPInitConfig. All of them happen because ConfGet returns 1 even if the value is NULL(due to misconfiguration for example).
This commit introduces a new function "ConfGetValue". It adds return values for NULL-pointer to ConfGet and could be used as a replacement for ConfGet.
Note: Simply modify ConfGet might not be a good idea, because there are some places where ConfGet should return 1 even if "value" is NULL. For example if ConfGet should get a Config-Leave in the yaml-hierarchy.
conf: multiple NULL-pointer dereferences in FlowInitConfig
This commit fixes multiple NULL-pointer dereferences in FlowInitConfig after reading in config-values(flow.hash-size, flow.prealloc and flow.memcap) for flow. Here is a sample ASAN-output:
=================================================================
ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fea73456646 bp 0x7fffd70e1ba0 sp 0x7fffd70e1328 T0)
0 0x7fea73456645 in strlen (/lib/x86_64-linux-gnu/libc.so.6+0x80645)
1 0x7fea76c98eec (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x3beec)
2 0x5643efb4c205 in FlowInitConfig /root/suricata-1/src/flow.c:455
3 0x5643efcd1751 in PreRunInit /root/suricata-1/src/suricata.c:2247
4 0x5643efcd49f4 in PostConfLoadedSetup /root/suricata-1/src/suricata.c:2748
5 0x5643efcd5402 in main /root/suricata-1/src/suricata.c:2884
6 0x7fea733f62b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
7 0x5643ef8761a9 in _start (/usr/local/bin/suricata+0xc51a9)
conf: use of NULL-pointer in DetectLoadCompleteSigPath
The "sig_file" argument of DetectLoadCompleteSigPath() is not checked for NULL-values. If this argument is NULL a SEGV occurs because of a dereferenced NULL-pointer in strlen in PathIsAbsolute. This commit fixes bug #2347. Here is the ASAN-output:
==17170==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd1afa00646 bp 0x7ffe8398e6d0 sp 0x7ffe8398de58 T0)
0 0x7fd1afa00645 in strlen (/lib/x86_64-linux-gnu/libc.so.6+0x80645)
1 0x7fd1b3242eec (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x3beec)
2 0x5561c8cddf7f in PathIsAbsolute /root/suricata-1/src/util-path.c:40
3 0x5561c8cddfea in PathIsRelative /root/suricata-1/src/util-path.c:65
4 0x5561c89275e4 in DetectLoadCompleteSigPath /root/suricata-1/src/detect.c:264
5 0x5561c8929e75 in SigLoadSignatures /root/suricata-1/src/detect.c:486
6 0x5561c8c0f2b3 in LoadSignatures /root/suricata-1/src/suricata.c:2419
7 0x5561c8c1051d in PostConfLoadedDetectSetup /root/suricata-1/src/suricata.c:2550
8 0x5561c8c12424 in main /root/suricata-1/src/suricata.c:2887
9 0x7fd1af9a02b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
10 0x5561c87b31a9 in _start (/usr/local/bin/suricata+0xc51a9)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0x80645) in strlen
conf: NULL-pointer dereference in ConfUnixSocketIsEnable
The value for the configuration-option "unix-command.enabled" is not properly checked in ConfUnixSocketIsEnable. This causes a NULL-pointer dereference in strcmp. This commit fixes bug #2346. The ASAN-output looks like:
ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f03b69737cc bp 0x7ffcef322c10 sp 0x7ffcef322390 T0)
0 0x7f03b69737cb (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x447cb)
1 0x5617a76d3f55 in ConfUnixSocketIsEnable /root/suricata-1/src/util-conf.c:104
2 0x5617a741b6e7 in DetectEngineMultiTenantSetup /root/suricata-1/src/detect-engine.c:2447
3 0x5617a769e0c3 in PostConfLoadedDetectSetup /root/suricata-1/src/suricata.c:2527
4 0x5617a76a0424 in main /root/suricata-1/src/suricata.c:2887
5 0x7f03b30c82b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
6 0x5617a72411a9 in _start (/usr/local/bin/suricata+0xc51a9)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x447cb
There is a memory-leak in DetectAddressTestConfVars. If the programm takes the "goto error"-path, the pointers gh and ghn will not be freed. This commit fixes bug #2345. Here is the ASAN-output:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
0 0x7f4347cb1d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
1 0x55fe1fc8dcfc in DetectAddressHeadInit /root/suricata-1/src/detect-engine-address.c:1534
2 0x55fe1fc8c50a in DetectAddressTestConfVars /root/suricata-1/src/detect-engine-address.c:1306
3 0x55fe1ff356bd in PostConfLoadedSetup /root/suricata-1/src/suricata.c:2696
4 0x55fe1ff365eb in main /root/suricata-1/src/suricata.c:2884
5 0x7f43443892b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
Direct leak of 24 byte(s) in 1 object(s) allocated from:
0 0x7f4347cb1d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
1 0x55fe1fc8dcfc in DetectAddressHeadInit /root/suricata-1/src/detect-engine-address.c:1534
2 0x55fe1fc8c524 in DetectAddressTestConfVars /root/suricata-1/src/detect-engine-address.c:1310
3 0x55fe1ff356bd in PostConfLoadedSetup /root/suricata-1/src/suricata.c:2696
4 0x55fe1ff365eb in main /root/suricata-1/src/suricata.c:2884
5 0x7f43443892b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
SUMMARY: AddressSanitizer: 48 byte(s) leaked in 2 allocation(s).
Victor Julien [Thu, 7 Dec 2017 16:47:03 +0000 (17:47 +0100)]
output: don't deadlock on log reopen failure
If output log reopen fails, don't try to output the error. This would
lead to a deadlock as reopen was called from a SCLogMessage call. This
call already held the output lock.
Victor Julien [Sun, 5 Nov 2017 10:37:48 +0000 (11:37 +0100)]
detect: content limits propagation
Propagate inspection limits from anchered keywords to the rest of
a rule.
Examples:
content:"A"; depth:1; is anchored, it can only match in the first byte
content:"A"; depth:1; content:"BC"; distance:0; within:2;
"BC" can only be in the 2nd and 3rd byte of the payload. So effectively
it has an implicite offset of 1 and an implicit depth of 3.
content:"A"; depth:1; content:"BC"; distance:0; can assume offset:1; for
the 2nd content.
content:"A"; depth:1; pcre:"/B/R"; content:"C"; distance:0; can assume
at least offset:1; for content "C". We can't analyzer the pcre pattern
(yet), so we assume it matches with 0 bytes.
Pcap file mode that when passed a directory will process all files in
that directory. If --pcap-file-continuous or continuous option is passed
in json, the directory will be monitored until the directory is
moved/deleted, suricata is interrupted, or the pcap-interrupt command
is used with unix command socket. Existing file implementation and new
directory implementation has moved from source-pcap-file into
pcap-file-helper and pcap-directory-helper.
Engine state will not reset between files.
Also satisfies:
* https://redmine.openinfosecfoundation.org/issues/2299
* https://redmine.openinfosecfoundation.org/issues/724
* https://redmine.openinfosecfoundation.org/issues/1476
Co-Authors: Dana Helwig <dana.helwig@protectwise.com> and
Danny Browning <danny.browning@protectwise.com>
Giuseppe Longo [Tue, 2 Aug 2016 14:09:41 +0000 (16:09 +0200)]
unix-manager: block live reload when -s/-S is specified
Currently, when live reload is executed through
unix-socket, suri prints in the console the following
error message:
"Live rule reload not possible if -s or -S option used at runtime."
Instead, prints "done" in unix socket,
when the live reload is not executed.
Add a non blocking function to reload rules. It will be useful
for remote system management to avoid to block them waiting the
reload. And as we now have a last-reload command we can get the
status of the current reload.
Giuseppe Longo [Wed, 4 May 2016 15:13:39 +0000 (17:13 +0200)]
detect-engine: remove DONE state
Remove the DONE state to fix a problem with state not being
changed correctly when multiple reload were done. As DONE was
not really useful, we can remove it.
Ralph Broenink [Sat, 14 Oct 2017 10:19:33 +0000 (12:19 +0200)]
doc: Restructure ToC
* All sections up to 2 levels deep are now shown regardless of whether they are a separate page
* Rename Xbits and Thresholding for more consistent naming
* Minor adjustment in the Payload Keywords section
Ralph Broenink [Sat, 14 Oct 2017 09:37:42 +0000 (11:37 +0200)]
doc: Replace images of tables and rules with text in rules docs
In some chapters of the rules documentation, many sections used examples of rules, but these were inserted into images. These have been replaced by text and HTML emphasis.
Additionally, some tables embedded into images were also replaced by reST tables.
conf: stack-based buffer-overflow in ParseFilename
There is a stack-based buffer-overflow in ParseFilename. Since the length of "outputs.pcap-log.filename" is not checked and the destination buffer "str" has a fixed length of 512 bytes, a buffer overflow happens with long filenames. An attacker could exploit this for code execution if the configuration-file is not protected properly. This commit fixes ticket #2335
This is what the asan-output looks like:
~/suricata-1/src# suricata -T -c ./suricata.yaml
[27871] 3/12/2017 -- 20:48:13 - (suricata.c:1876) <Info> (ParseCommandLine) -- Running suricata under test mode
[27871] 3/12/2017 -- 20:48:13 - (suricata.c:1109) <Notice> (LogVersion) -- This is Suricata version 4.0.0-dev (rev f3fea60b)
=================================================================
==27871==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffbe9d75e0 at pc 0x55897b5f935f bp 0x7fffbe9d72b0 sp 0x7fffbe9d72a8
WRITE of size 1 at 0x7fffbe9d75e0 thread T0 (Suricata-Main)
0 0x55897b5f935e in ParseFilename /root/suricata-1/src/log-pcap.c:895
1 0x55897b5fb173 in PcapLogInitCtx /root/suricata-1/src/log-pcap.c:985
2 0x55897b6af103 in RunModeInitializeOutputs /root/suricata-1/src/runmodes.c:752
3 0x55897b72c6b5 in PreRunPostPrivsDropInit /root/suricata-1/src/suricata.c:2263
4 0x55897b730416 in main /root/suricata-1/src/suricata.c:2898
5 0x7f947f6db2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
6 0x55897b2d4c19 in _start (/usr/local/bin/suricata+0xc4c19)
Address 0x7fffbe9d75e0 is located in stack of thread T0 (Suricata-Main) at offset 672 in frame
0 0x55897b5f7fcc in ParseFilename /root/suricata-1/src/log-pcap.c:836
This frame has 3 object(s):
[32, 104) 'toks'
[160, 672) 'str' <== Memory access at offset 672 overflows this variable
[704, 2752) '_sc_log_msg'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /root/suricata-1/src/log-pcap.c:895 in ParseFilename
Shadow bytes around the buggy address:
0x100077d32e60: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
0x100077d32e70: 00 00 00 00 00 f4 f4 f4 f2 f2 f2 f2 00 00 00 00
0x100077d32e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32e90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32ea0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100077d32eb0: 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 f2
0x100077d32ec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32ed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32ee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100077d32f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==27871==ABORTING
Gaurav Singh [Tue, 17 Oct 2017 01:01:53 +0000 (18:01 -0700)]
Adds options to mark when a file is final.
This takes the form of an option to add the pid of the process to file
names. Additionally, it adds a suffix to the file name to indicate it is
not finalized.
Adding the pid to the file name reduces the likelihood that a file is
overwritten when suricata is unexpectedly killed. The number in the
waldo file is only written out during a clean shutdown. In the event
of an improper shutdown, extracted files will be written using the old
number and existing files with the same name will be overwritten.
Writes extracted files and their metadata to a temporary file suffixed
with '.tmp'. Renames the files when they are completely done being
written. As-is there is no way to know that a file on disk is still
being written to by suricata.
Pierre Chifflier [Thu, 26 Oct 2017 06:05:41 +0000 (08:05 +0200)]
rust/ntp: convert parser to new registration method
Converting the NTP parser to the new registration method is a simple,
3-steps process:
- change the extern functions to use generic input parameters (functions
in all parsers must share common types to be generic) and cast them
- declare the Parser structure
- remove the C code and call the registration function