netfilter buglog - Aug 2011 - [Bug 738] New: reading beyond buffer limits in nf_conntrack_proto_tcp.c::tcp_options()

http://bugzilla.netfilter.org/show_bug.cgi?id=738

           Summary: reading beyond buffer limits in
                    nf_conntrack_proto_tcp.c::tcp_options()
           Product: netfilter/iptables
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: minor
          Priority: P2
         Component: nf_conntrack
        AssignedTo: netfilter-buglog at lists.netfilter.org
        ReportedBy: mbuilov at gmail.com
   Estimated Hours: 0.0


Incorrect handling of invalid TCP option with too big opsize may lead to read
access beyond tcp-packet or buffer allocated on stack.

in net/netfilter/nf_conntrack_proto_tcp.c:

 397     while (length > 0) {
 398         int opcode=*ptr++;
 399         int opsize;
 400
 401         switch (opcode) {
 402         case TCPOPT_EOL:
 403             return;
 404         case TCPOPT_NOP:    /* Ref: RFC 793 section 3.1 */
 405             length--;
 406             continue;
 407         default:
 408             opsize=*ptr++;
 409             if (opsize < 2) /* "silly options" */
 410                 return;
 411             if (opsize > length)
 412                 break;  /* don't parse partial options */
....
 428             ptr += opsize - 2;
 429             length -= opsize;
 430         }
 431     }

doing 'break' at line 412 we forget to decrement 'length'.

Also, there is a question: why 'break' and not just 'return'?
Comment for tcp_options() says that it is a "Simplified tcp_parse_options
routine from tcp_input.c", but tcp_parse_options() does 'return' in
case of
"partial options".


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738





--- Comment #1 from Michael M. Builov <mbuilov at gmail.com>  2011-08-21
19:13:42 ---
the same 'break' also found in similar option parsing while() of
tcp_sack() at
linux-3.0.1/net/netfilter/nf_conntrack_proto_tcp.c:472


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738





--- Comment #2 from Michael M. Builov <mbuilov at gmail.com>  2011-08-21
20:00:22 ---
sorry, one more note for linux-3.0.1/net/netfilter/nf_conntrack_proto_tcp.c not
related to reported bug:

 434 static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff,
 435                      const struct tcphdr *tcph, __u32 *sack)
 436 {
 437     unsigned char buff[(15 * 4) - sizeof(struct tcphdr)];
 438     const unsigned char *ptr;
 439     int length = (tcph->doff*4) - sizeof(struct tcphdr);
....
 441
 449     /* Fast path for timestamp-only option */
 450     if (length == TCPOLEN_TSTAMP_ALIGNED*4

at line 450 there is wrong multiplication of TCPOLEN_TSTAMP_ALIGNED by 4.
Maximum length value is 40, but TCPOLEN_TSTAMP_ALIGNED*4 == 12*4 == 48.


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738





--- Comment #3 from Michael M. Builov <mbuilov at gmail.com>  2011-08-22
01:05:26 ---
and in linux-3.0.3/net/netfilter/nf_conntrack_proto_tcp.c at lines 408 and 469
of 'length' is not checked before reading 'optsize'
('length' should be >= 2):

 407         default:
 408             opsize=*ptr++;
 409             if (opsize < 2) /* "silly options" */
...
 467         default:
 468             opsize = *ptr++;
 469             if (opsize < 2) /* "silly options" */

as like in linux-3.0.3/net/ipv4/tcp_input.c at line 3768:

3767         default:
3768             opsize = *ptr++;
3769             if (opsize < 2) /* "silly options" */
3770                 return;

This is definitely access out of malformed packet bounds.
May be not fatal, but looks inaccurate.


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738


Jan Engelhardt <jengelh at medozas.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jengelh at medozas.de
         AssignedTo|netfilter-                  |kaber at trash.net
                   |buglog at lists.netfilter.org  |




-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738





--- Comment #4 from Patrick McHardy <kaber at trash.net>  2011-08-30
14:14:50 ---
I've asked Jozsef to have a look at this.


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=738


Jozsef Kadlecsik <kadlec at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kadlec at netfilter.org
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #5 from Jozsef Kadlecsik <kadlec at netfilter.org>  2011-08-30
15:25:53 ---
All of your findings are valid bugs in the options handling in the TCP
conntrack code. I have just prepared the patches and about to send to
netfilter-devel.


-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.