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

[Jon Siwek (JIRA)]

     [ 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)
                   2.5

> 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
(v6.4-OD-15-055#64014)