[lwip-devel] [bug #48402] Bug in skipping over TCP options

[Ambroz Bizjak]

URL:
  <http://savannah.nongnu.org/bugs/?48402>

                 Summary: Bug in skipping over TCP options
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: abizjak
            Submitted on: Mon 04 Jul 2016 09:30:55 PM GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: git head

    _______________________________________________________

Details:

Hey,
I am looking at the code in tcp_in.c that attempts to skip over the TCP header
including TCP options (/* Move the payload pointer in the pbuf ) and I think
there it has the following issues:

1) LWIP_ASSERT("tcphdr_opt1len >= p->len"...) can actually be >, because  that
follows from "if (p->len < hdrlen_bytes) {".

2) "if (optlen > tcphdr_opt1len) {" is always true since it is equivalent to
my suggested improved assert above, i.e. to "tcphdr_opt1len > p->len" at the
time of that assert.

3) The check "if(tcphdr_optlen - tcphdr_opt1len > p->tot_len) {" verifies only
that the options fit into the pbuf chain, but the following pbuf_header call
is done without a check that the amount the pbuf is advanced does not exceed
the size of the (second) pbuf. This is a bug because pbuf_header will hit
LWIP_ERROR("increment_magnitude <= p->len"....) which will call
LWIP_PLATFORM_ASSERT if assertions are enabled.

4) This code is unnecessarily complicated, making it hard to see that my
previous points are true (or false :). Too many variables especially
"optlens", lots of mutations of such variables!




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?48402>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/