From c16509a8b611314b5760a8b7d0e4330ac6f50ffc Mon Sep 17 00:00:00 2001 From: Wolfgang Hotwagner Date: Wed, 6 Dec 2017 11:12:42 +0000 Subject: [PATCH] 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) (ParseCommandLine) -- Running suricata under test mode [27871] 3/12/2017 -- 20:48:13 - (suricata.c:1109) (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 --- src/log-pcap.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/log-pcap.c b/src/log-pcap.c index cd8acc164b..869c0ff3fe 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -106,6 +106,7 @@ typedef struct PcapLogProfileData_ { } PcapLogProfileData; #define MAX_TOKS 9 +#define MAX_FILENAMELEN 513 /** * PcapLog thread vars @@ -832,16 +833,24 @@ static TmEcode PcapLogDataDeinit(ThreadVars *t, void *thread_data) return TM_ECODE_OK; } + static int ParseFilename(PcapLogData *pl, const char *filename) { char *toks[MAX_TOKS] = { NULL }; int tok = 0; - char str[512] = ""; + char str[MAX_FILENAMELEN] = ""; int s = 0; int i, x; char *p = NULL; + size_t filename_len = 0; if (filename) { + filename_len = strlen(filename); + if (filename_len > (MAX_FILENAMELEN-1)) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "invalid filename option. Max filename-length: %d",MAX_FILENAMELEN-1); + goto error; + } + for (i = 0; i < (int)strlen(filename); i++) { if (tok >= MAX_TOKS) { SCLogError(SC_ERR_INVALID_ARGUMENT,