BroString::Set() Attempts Allocation of Negative-Length Memory

Description

When the tcp_packet() event is used, Bro may attempt to allocate memory that is negative in length (i.e. -6 bytes). Bro crashes with the following output:

tcmalloc: large alloc 0 bytes == (nil) @ 0x7f6abeaefc73 0x7f6abeb111c3 0x765e81 0x765b24 0x872562 0xaddc2f 0xaded94 0xb7aeca 0x775180 0x84105b 0x83f5c0 0x83f39d 0x7fb1bc 0xb3cde6 0x7fb3d9 0x750e98 0x7f6abdaf4ec5 0x72e553 (nil)
out of memory in new.
1103139821.634774 fatal error: out of memory in new.

The attached pcap file and bro script cause such a crash when run with the following command:

/usr/local/bro/bin/bro -r lbl-internal.20041215-1142.port004.dump.anon /usr/local/bro/share/bro/site/negativeMemory.bro

A core file is not being generated for me, despite following the directions for reporting problems (https://www.bro.org/support/reporting-problems.html#getting-more-information-after-acrash). The file named memory_trace.log shows an alternatively formatted traceback of the stack when the error occurs.

Environment

Linux Mint 17.1 (Ubuntu 14.04) on bare metal and in a VirtualBox VM.
Mac OS X 10.10.3

Activity

Show:
Jonathan Ganz
June 17, 2015, 5:07 PM
Edited

I have added the gdb backtrace (backtrace.log). It indicates that the (payload?) length passed is -6, which causes issues when attempting to allocate that much memory. A simple workaround I found was to take the absolute value of len in function BroString::Set() (bro-2.4/src/BroString.cc:125). This might cause other problems though (it assumes that the the negative value should just be a positive value of the same magnitude) and does not address the root cause. It may be that the packet is malformed, but Bro should be able to fail more gracefully than this.

Aaron Brown
June 18, 2015, 8:29 PM

Looks like ExtractTCP_Header makes sure that the captured length is greater than the struct tcphdr size instead of the actual header length (th_off * 4). It then subtracts the actual header length, so you end up with a negative number (caplen is 36, but the len and header length is 40) instead of getting a "truncated header" error. The easiest fix would be to just replace this in TCP.cc at line ~442:

sizeof(struct tcphdr) > uint32(caplen) )

with:

tcp_hdr_len > uint32(caplen) )

But i'm not positive there isn't some better way this check should be done.

Assignee

Robin Sommer

Reporter

Jonathan Ganz

Labels

External issue ID

None

Components

Fix versions

Affects versions

Priority

Normal
Configure