[Bro-Dev] Incorrect bounds checking with truncated TCP options

Gregor Maier gregor at icir.org
Mon Dec 13 16:06:26 PST 2010


Hi,

there is a potential problem in Bro when it receives a packet with
truncated TCP options (i.e., the packet isn't long enough to accommodate
all options).

This can happen:

a) in the ConnCompressor: it calls ParseTCPOptions without checking
   whether the packet is long enough to contain all options.
   ConnCompressor needs to parse the TCP options to know the window
   scaling factor.


b) in TCP.cc, when caplen < len and len is long enough for the TCP
options but caplen is not.

ExtractTCPHeader() ensures that the len is long enough to contain the
options and that caplen is long enough to contain a struct tcphdr (but
doesn't check for options). Presumably this is done to enable parsing of
header only traces that truncate options.

Nevertheless, the TCP Analyzer correctly checks caplen before
ParseTCPOptions().

But there are also situations when options are parsed without checking
for caplen:
 * BuildSYNVal(), which is called on every SYN to get the window
   scaling options.
 * BuildOSVal(), which is only called when the OS_version_found event
   has a handler
 * TCP TraceRewriter (presumably we can ignore this, as we were going
   to remove it anyway)


So, question is: what's the best way to tackle this? One option is to
not parse packets that are truncated. But that's probably not a good
idea wrt header traces.
The other option is to check for the caplen whenever we parse options.
That might be cumbersome as this information needs to be passed to many
functions, e.g. in TCP_Analyzer: ProcessFlags -> ProcessSYN ->
BuildSYNPacketVal.

(In any case, truncated packets mean that we can't learn the window
scaling....)


cu
Gregor
-- 
Gregor Maier                                             gregor at icir.org
Int. Computer Science Institute (ICSI)          gregor at icsi.berkeley.edu
1947 Center St., Ste. 600                    http://www.icir.org/gregor/
Berkeley, CA 94704
USA


More information about the bro-dev mailing list