Commit Graph

68 Commits (873bc290bc010fa24eed9b2953ebc7a8a6ef2e2e)

Author SHA1 Message Date
Max Fillinger 081fdc6804 pcap-log: Don't leak memory in LZ4 error paths 7 years ago
Victor Julien 00111499b0 log-pcap: fix coverity memleak warning 8 years ago
Victor Julien d0a8310fcf pcap-log: don't divide by 0 on no traffic 8 years ago
Max Fillinger b85a0b188b Add an option for compressing pcap-log files
Introduces the option 'outputs.pcap-log.compression' which can be set
to 'none' or 'lz4', plus options to set the compression level and to
enable checksums. SCFmemopen is used to make pcap_dump() write to a
buffer which is then compressed using liblz4.
8 years ago
Victor Julien a5de9968dd gcc8: fix format truncation warnings 8 years ago
Jason Ish 00e6cd4ced output: introduce init return type
The new OutputInitResult is a struct return type that allows
logger init functions to return a NULL context without
raising error.

Instead of returning NULL to signal error, the "ok" field will
be set to false. If ok, but the ctx is NULL, then silently
move on to the next logger.

Use case: multiple versions of a specific logger, and one
implementation decides the configuration is not for that
implemenation. It can return NULL, ok.
9 years ago
Victor Julien d8ddd3b5bc mingw: work around mingw mkdir
mingw doesn't come with a posix compliant mkdir as it only takes
a single argument.
9 years ago
Wolfgang Hotwagner c16509a8b6 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
9 years ago
Victor Julien 16845d8c92 pcap-log: fix path construct check 9 years ago
Victor Julien 96b2e8afc0 gcc7: fixes for format string warnings
GCC 7.1.1 on Fedora gave several warnings with -Wimplicit-fallthrough
and -Wformat-truncation

This patch addresses the warnings.
9 years ago
Victor Julien ab1200fbd7 compiler: more strict compiler warnings
Set flags by default:

    -Wmissing-prototypes
    -Wmissing-declarations
    -Wstrict-prototypes
    -Wwrite-strings
    -Wcast-align
    -Wbad-function-cast
    -Wformat-security
    -Wno-format-nonliteral
    -Wmissing-format-attribute
    -funsigned-char

Fix minor compiler warnings for these new flags on gcc and clang.
9 years ago
Jason Ish 19e578a740 pcap-log: fix pcre_study error check
Code was failing on a NULL return value which can be returned
when there was nothing todo instead of an error. Instead
check the errbuf for a non-NULL value to determine error.
10 years ago
Jason Ish 27ec811187 pcap-log: fix memory leak during initialization of ring buffer
A free was missing when files are removed during initialization
of the ring buffer.

Redmine issue:
https://redmine.openinfosecfoundation.org/issues/1985
10 years ago
Jason Ish 79a3c6c7b1 log-pcap: use a snaplen of 262144 instead of -1.
Newer versions of libpcap will not open pcap files with a
snaplen of -1, instead use the current maximum value of
262144.

Issue:
https://redmine.openinfosecfoundation.org/issues/1987
10 years ago
Jason Ish a1eca40611 log-pcap.c: cleanup scan-build warning
Don't initialize value to a value that is never used.
10 years ago
Jason Ish 553f7ec290 log-pcap.c: fix resource leak found by coverity
Goto the failure label instead of returning which will allow the open
directory to get cleaned up.

Fixes:

*** CID 1394675:  Resource leaks  (RESOURCE_LEAK)
/src/log-pcap.c: 615 in PcapLogInitRingBuffer()
609                  * failure as the file might just not be a pcap log file. */
610                 continue;
611             }
612
613             PcapFileName *pf = SCCalloc(sizeof(*pf), 1);
614             if (unlikely(pf == NULL)) {
>>>     CID 1394675:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "dir" going out of scope leaks the storage it points to.
615                 return TM_ECODE_FAILED;
616             }
617             char path[PATH_MAX];
618             snprintf(path, PATH_MAX - 1, "%s/%s", pattern, entry->d_name);
619             if ((pf->filename = SCStrdup(path)) == NULL) {
620                 goto fail;

This also means that pf can be NULL which should clear up CID
1394676 (REVERSE_INULL).
10 years ago
Jason Ish bbb93e487e pcap-log: seed ring buffer on start up
On start, look for existing pcap log files and add them to
the ring buffer. This makes pcap-log self maintaining over
restarts removing the need for external tools to clear
orphaned files.
10 years ago
Jason Ish 09c3e1dd8a pcap-log: cleanup allocations at exit
Particularly in multi-mode, allocations made for each thread were
not being cleaned.

ASAN reports no leaks now on exit.
10 years ago
Jason Ish 1b4ba4496c logging: rename registration functions to not have tmm
As the logging modules are no longer threading modules, rename
them so they don't look like they are being registered as
threading modules.

Also, move the registration to the output.c which will handle
registration of the loggers.
10 years ago
Jason Ish 04a44a077d logging: convert pcap log to non-thread module 10 years ago
maxtors 69863f7b1c Corrected and unified debugmessages for init data errors in *ThreadInit. 10 years ago
Victor Julien d44cb3f6fe pcap-log: add option to honor pass rules
Add option (disabled by default) to honor pass rules. This means that
when a pass rule matches in a flow, it's packets are no longer stored
by the pcap-log module.
12 years ago
Ken Steele 8f1d75039a Enforce function coding standard
Functions should be defined as:

int foo(void)
{
}

Rather than:
int food(void) {
}

All functions where changed by a script to match this standard.
12 years ago
Victor Julien 0c1696f84b pcap-log: unify lock handling, fixes Coverity warn
*** CID 1229124:  Data race condition  (MISSING_LOCK)
/src/log-pcap.c: 363 in PcapLog()
357         {
358             return TM_ECODE_OK;
359         }
360
361         PcapLogLock(pl);
362
>>>     CID 1229124:  Data race condition  (MISSING_LOCK)
>>>     Accessing "pl->pkt_cnt" without holding lock "PcapLogData_.plog_lock". Elsewhere, "PcapLogData_.pkt_cnt" is accessed with "PcapLogData_.plog_lock" held 1 out of 2 times (1 of these accesses strongly imply that it is necessary).
363         pl->pkt_cnt++;
364         pl->h->ts.tv_sec = p->ts.tv_sec;
365         pl->h->ts.tv_usec = p->ts.tv_usec;
366         pl->h->caplen = GET_PKT_LEN(p);
367         pl->h->len = GET_PKT_LEN(p);
368         len = sizeof(*pl->h) + GET_PKT_LEN(p);
12 years ago
Victor Julien be1979b2f9 pcap-log: support dynamic file names in multi
When using multi mode, the filename can use a few variables:

%n -- thread number, where the 1st thread has 1, and it increments
%i -- thread id (system thread id, similar to pid)
%t -- timestamp, where seconds or seconds+usecs depends on
      the ts-format option.

Example:
filename: filename: pcaps/%n/pcap.%t
This will translate to: pcaps/3/pcap.1256792217 for the 3rd thread.

Note that while it's possible to use directories, they won't be
created. So make sure they exist.
12 years ago
Victor Julien 6cebe7ef7b pcap-log: performance optimizations
This patch adds a field 'is_private' to PcapLogData, so that the
using thread knows if it needs to lock access to it or not.

Reshuffle PcapLogData to roughly match order of access.
12 years ago
Victor Julien 923341fa05 pcap-log: implement multi mode
This patch implements a new mode in pcap-logging: 'multi'. It stores
a pcap file per logger thread, instead of just one file globally.

This removes lock contention, so it brings a lot more performance.

The trade off is that there are now mulitple files where there would
be one before.

Files have a thread id added to their name: base_name.tid.ts, so by
we have something like: "log.pcap.20057.1254500095".
12 years ago
Victor Julien 4922cd2d36 pcap-log: introduce PcapLogThreadData
PcapLog uses the global data structure PcapLogData as thread data
as well. This is possible because all operations on it are locked.

This patch introduces PcapLogThreadData. It contains a pointer to
the PcapLogData. Currently to the global instance, but in the future
it may hold a thread-local instance of PcapLogData.
12 years ago
Victor Julien bbc8c1ea05 log-pcap: multi mode yaml parsing
In preparation of the multi file mode, add 'multi' as a value to
the mode.
12 years ago
Victor Julien cf4db47931 log-pcap: lock profiling
Add lock profiling to pcap logging profiling.
12 years ago
Victor Julien adde58d2cb log-pcap: improve profiling
Add profiling to a logfile. Default is $log_dir/pcaplog_stats.log

The counters for open, close, rotate, write and handles are written
to it, as well as:
- total bytes written
- cost per MiB
- cost per GiB

Option is disabled by default.
12 years ago
Victor Julien 1af2f6528b log-pcap code cleanups
Code cleanups to make functions static.
12 years ago
Victor Julien fd7dd09f4c profiling: add pcap logger profiling
Tracks: file open, file close, file rotate (which includes open and
close), file write and open handles.

Open handles measures the cost of open the libpcap handles.
12 years ago
Eric Leblond 28c5c68192 error checking: add missing alloc error treatment
The return of some malloc like functions was not treated in some
places of the code.
13 years ago
Ken Steele d4dd18eb85 Clean up SCLocalTime() usage
Remove cast of return type from SCLocalTime() as it is not needed.
Replace last use of localtime_r() with SCLocalTime().
13 years ago
Anoop Saldanha d292f1a529 fix for #915. Fix segv when we send NULL to snprintf. 13 years ago
Eric Leblond 34abd818dd Prefix util-conf function with Config 13 years ago
Eric Leblond 54006de40c Use new function GetLogDirectory() 13 years ago
Eric Leblond 786cbb1244 log-pcap: don't limit snaplen. 14 years ago
Last G 8ae11f73b2 Added parentheses to fix Eclipse static code analysis
Fixed bug in action priority (REJECT_DST had lowest prio)
14 years ago
Victor Julien 829238e49c OpenBSD 5.2 build fixes, Unit test fix. 14 years ago
Eric Leblond e176be6fcc Use unlikely for error treatment.
When handling error case on SCMallog, SCCalloc or SCStrdup
we are in an unlikely case. This patch adds the unlikely()
expression to indicate this to gcc.

This patch has been obtained via coccinelle. The transformation
is the following:

@istested@
identifier x;
statement S1;
identifier func =~ "(SCMalloc|SCStrdup|SCCalloc)";
@@

x = func(...)
... when != x
- if (x == NULL) S1
+ if (unlikely(x == NULL)) S1
14 years ago
Victor Julien 8cd460dde5 Don't display a warning when log-pcap tries to remove an already removed file. 14 years ago
Eric Leblond a0e57f58e5 OpenBSD: introduce SCLocalTime function.
This function is a wrapper to localtime_r. It is needed to avoid
a compilation warning on OpenBSD. I'm forced to type the function
to a non pointer first parameter. If not we will have to use two
differents functions in OpenBSD where tv->tv_sec is a long
(different from time_t).
14 years ago
Anoop Saldanha 6c5b596ada coverity fixes 14 years ago
Anoop Saldanha f1863370a5 clean log pcap 14 years ago
Anoop Saldanha 6392202872 restructure log pcap to use a different setup, which is resilient to thread failure/restarts 14 years ago
Anoop Saldanha fd21b83f3e don't return TM failure on failing to remove log file 14 years ago
Anoop Saldanha 4bc907414b init every new pf instance in log pcap 14 years ago
Victor Julien 372ab9c433 Another batch of minor fixed for issues found by Coverity. 15 years ago