[Bro-Dev] [JIRA] (BIT-351) Incorrect bounds checking with truncated TCP options

Jon Siwek (JIRA) jira at bro-tracker.atlassian.net
Tue Mar 17 10:08:00 PDT 2015

     [ https://bro-tracker.atlassian.net/browse/BIT-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jon Siwek updated BIT-351:
    Fix Version/s:     (was: 2.4)

> Incorrect bounds checking with truncated TCP options
> ----------------------------------------------------
>                 Key: BIT-351
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-351
>             Project: Bro Issue Tracker
>          Issue Type: Problem
>          Components: Bro
>    Affects Versions: git/master
>            Reporter: gregor
>             Fix For: 2.5
> {noformat}
> #!rst
> (from an e-mail I sent a while ago)
> (setting milestone to 1.6. Can probably be pushed back)
> 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....)
> {noformat}

This message was sent by Atlassian JIRA

More information about the bro-dev mailing list