fixed bug #37969 SYN packet dropped as short packet in tcp_input function

CHANGELOG

@@ -165,6 +165,10 @@ HISTORY
 
  ++ Bugfixes:
 
+  2014-06-03: Simon Goldschmidt
+  * tcp_impl.h, tcp_in.c: fixed bug #37969 SYN packet dropped as short packet in
+    tcp_input function
+
   2014-05-20: Simon Goldschmidt
   * tcp_out.c: fixed bug #37184 tcp_write problem for pcbs in the SYN_SENT state
 

src/core/tcp_in.c

@@ -67,6 +67,9 @@
    function. */
 static struct tcp_seg inseg;
 static struct tcp_hdr *tcphdr;
+static u16_t tcphdr_opt1len;
+static u8_t* tcphdr_opt2;
+static u16_t tcp_optidx;
 static u32_t seqno, ackno;
 static u8_t flags;
 static u16_t tcplen;
@@ -150,11 +153,39 @@ tcp_input(struct pbuf *p, struct netif *inp)
   /* Move the payload pointer in the pbuf so that it points to the
      TCP data instead of the TCP header. */
   hdrlen = TCPH_HDRLEN(tcphdr);
-  if(pbuf_header(p, -(hdrlen * 4))){
+  tcphdr_opt1len = (hdrlen * 4) - TCP_HLEN;
-    /* drop short packets */
+  tcphdr_opt2 = NULL;
-    LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet\n"));
+  if (p->len < hdrlen * 4) {
-    TCP_STATS_INC(tcp.lenerr);
+    if (p->len >= TCP_HLEN) {
-    goto dropped;
+      /* TCP header fits into first pbuf, options don't - data is in the next pbuf */
+      u16_t optlen = tcphdr_opt1len;
+      pbuf_header(p, -TCP_HLEN); /* cannot fail */
+      LWIP_ASSERT("tcphdr_opt1len >= p->len", tcphdr_opt1len >= p->len);
+      LWIP_ASSERT("p->next != NULL", p->next != NULL);
+      tcphdr_opt1len = p->len;
+      if (optlen > tcphdr_opt1len) {
+        s16_t opt2len;
+        /* options continue in the next pbuf: set p to zero length and hide the
+           options in the next pbuf (adjusting p->tot_len) */
+        u8_t phret = pbuf_header(p, -(s16_t)tcphdr_opt1len);
+        LWIP_ASSERT("phret == 0", phret == 0);
+        tcphdr_opt2 = (u8_t*)p->next->payload;
+        opt2len = optlen - tcphdr_opt1len;
+        phret = pbuf_header(p->next, -opt2len);
+        LWIP_ASSERT("phret == 0", phret == 0);
+        /* p->next->payload now points to the TCP data */
+        /* manually adjust p->tot_len to changed p->next->tot_len change */
+        p->tot_len -= opt2len;
+      }
+      LWIP_ASSERT("p->len == 0", p->len == 0);
+    } else {
+      /* drop short packets */
+      LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet\n"));
+      TCP_STATS_INC(tcp.lenerr);
+      goto dropped;
+    }
+  } else {
+    pbuf_header(p, -(hdrlen * 4)); /* cannot fail */
   }
 
   /* Convert fields in TCP header to host byte order. */
@@ -1611,6 +1642,17 @@ tcp_receive(struct tcp_pcb *pcb)
   }
 }
 
+static u8_t tcp_getoptbyte(void)
+{
+  if ((tcphdr_opt2 == NULL) || (tcp_optidx < tcphdr_opt1len)) {
+    u8_t* opts = (u8_t *)tcphdr + TCP_HLEN;
+    return opts[tcp_optidx++];
+  } else {
+    u8_t idx = tcp_optidx++ - tcphdr_opt1len;
+    return tcphdr_opt2[idx];
+  }
+}
+
 /**
  * Parses the options contained in the incoming segment.
  *
@@ -1622,78 +1664,74 @@ tcp_receive(struct tcp_pcb *pcb)
 static void
 tcp_parseopt(struct tcp_pcb *pcb)
 {
-  u16_t c, max_c;
+  u8_t data;
   u16_t mss;
-  u8_t *opts, opt;
 #if LWIP_TCP_TIMESTAMPS
   u32_t tsval;
 #endif
 
-  opts = (u8_t *)tcphdr + TCP_HLEN;
-
   /* Parse the TCP MSS option, if present. */
   if (TCPH_HDRLEN(tcphdr) > 0x5) {
-    max_c = (TCPH_HDRLEN(tcphdr) - 5) << 2;
+    u16_t max_c = (TCPH_HDRLEN(tcphdr) - 5) << 2;
-    for (c = 0; c < max_c; ) {
+    for (tcp_optidx = 0; tcp_optidx < max_c; ) {
-      opt = opts[c];
+      u8_t opt = tcp_getoptbyte();
       switch (opt) {
-      case 0x00:
+      case LWIP_TCP_OPT_EOL:
         /* End of options. */
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: EOL\n"));
         return;
-      case 0x01:
+      case LWIP_TCP_OPT_NOP:
         /* NOP option. */
-        ++c;
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: NOP\n"));
         break;
-      case 0x02:
+      case LWIP_TCP_OPT_MSS:
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: MSS\n"));
-        if (opts[c + 1] != 0x04 || c + 0x04 > max_c) {
+        if (tcp_getoptbyte() != LWIP_TCP_OPT_LEN_MSS || (tcp_optidx - 2 + LWIP_TCP_OPT_LEN_MSS) > max_c) {
           /* Bad length */
           LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: bad length\n"));
           return;
         }
         /* An MSS option with the right option length. */
-        mss = (opts[c + 2] << 8) | opts[c + 3];
+        mss = (tcp_getoptbyte() << 8);
+        mss |= tcp_getoptbyte();
         /* Limit the mss to the configured TCP_MSS and prevent division by zero */
         pcb->mss = ((mss > TCP_MSS) || (mss == 0)) ? TCP_MSS : mss;
-        /* Advance to next option */
-        c += 0x04;
         break;
 #if LWIP_WND_SCALE
-      case 0x03:
+      case LWIP_TCP_OPT_WS:
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: WND_SCALE\n"));
-        if (opts[c + 1] != 0x03 || c + 0x03 > max_c) {
+        if (tcp_getoptbyte() != LWIP_TCP_OPT_LEN_WS || (tcp_optidx - 2 + LWIP_TCP_OPT_LEN_WS) > max_c) {
           /* Bad length */
           LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: bad length\n"));
           return;
         }
         /* If syn was received with wnd scale option,
            activate wnd scale opt */
+        data = tcp_getoptbyte();
         if (flags & TCP_SYN) {
           /* An WND_SCALE option with the right option length. */
-          pcb->snd_scale = opts[c + 2];
+          pcb->snd_scale = data;
           if (pcb->snd_scale > 14U) {
             pcb->snd_scale = 14U;
           }
           pcb->rcv_scale = TCP_RCV_SCALE;
           pcb->flags |= TF_WND_SCALE;
         }
-        /* Advance to next option */
-        c += 0x03;
         break;
 #endif
 #if LWIP_TCP_TIMESTAMPS
-      case 0x08:
+      case LWIP_TCP_OPT_TS:
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: TS\n"));
-        if (opts[c + 1] != 0x0A || c + 0x0A > max_c) {
+        if (tcp_getoptbyte() != LWIP_TCP_OPT_LEN_TS || (tcp_optidx - 2 + LWIP_TCP_OPT_LEN_TS) > max_c) {
           /* Bad length */
           LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: bad length\n"));
           return;
         }
         /* TCP timestamp option with valid length */
-        tsval = (opts[c+2]) | (opts[c+3] << 8) |
+        tsval = tcp_getoptbyte();
-          (opts[c+4] << 16) | (opts[c+5] << 24);
+        tsval |= (tcp_getoptbyte() << 8);
+        tsval |= (tcp_getoptbyte() << 16);
+        tsval |= (tcp_getoptbyte() << 24);
         if (flags & TCP_SYN) {
           pcb->ts_recent = ntohl(tsval);
           /* Enable sending timestamps in every segment now that we know
@@ -1702,13 +1740,14 @@ tcp_parseopt(struct tcp_pcb *pcb)
         } else if (TCP_SEQ_BETWEEN(pcb->ts_lastacksent, seqno, seqno+tcplen)) {
           pcb->ts_recent = ntohl(tsval);
         }
-        /* Advance to next option */
+        /* Advance to next option (6 bytes already read) */
-        c += 0x0A;
+        tcp_optidx += LWIP_TCP_OPT_LEN_TS - 6;
         break;
 #endif
       default:
         LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: other\n"));
-        if (opts[c + 1] == 0) {
+        data = tcp_getoptbyte();
+        if (data < 2) {
           LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_parseopt: bad length\n"));
           /* If the length field is zero, the options are malformed
              and we don't process them further. */
@@ -1716,7 +1755,7 @@ tcp_parseopt(struct tcp_pcb *pcb)
         }
         /* All other options have a length field, so that we easily
            can skip past them. */
-        c += opts[c + 1];
+        tcp_optidx += data - 2;
       }
     }
   }

src/include/lwip/tcp_impl.h

@@ -298,22 +298,30 @@ struct tcp_seg {
   struct tcp_hdr *tcphdr;  /* the TCP header */
 };
 
-#define LWIP_TCP_OPT_LEN_MSS  4
+#define LWIP_TCP_OPT_EOL        0
+#define LWIP_TCP_OPT_NOP        1
+#define LWIP_TCP_OPT_MSS        2
+#define LWIP_TCP_OPT_WS         3
+#define LWIP_TCP_OPT_TS         8
+
+#define LWIP_TCP_OPT_LEN_MSS    4
 #if LWIP_TCP_TIMESTAMPS
-#define LWIP_TCP_OPT_LEN_TS   12
+#define LWIP_TCP_OPT_LEN_TS     10
+#define LWIP_TCP_OPT_LEN_TS_OUT 12 /* aligned for output (includes NOP padding) */
 #else
-#define LWIP_TCP_OPT_LEN_TS   0
+#define LWIP_TCP_OPT_LEN_TS_OUT 0
 #endif
 #if LWIP_WND_SCALE
-#define LWIP_TCP_OPT_LEN_WS   4
+#define LWIP_TCP_OPT_LEN_WS     3
+#define LWIP_TCP_OPT_LEN_WS_OUT 4 /* aligned for output (includes NOP padding) */
 #else
-#define LWIP_TCP_OPT_LEN_WS   0
+#define LWIP_TCP_OPT_LEN_WS_OUT 0
 #endif
 
 #define LWIP_TCP_OPT_LENGTH(flags) \
-  (flags & TF_SEG_OPTS_MSS       ? LWIP_TCP_OPT_LEN_MSS : 0) + \
+  (flags & TF_SEG_OPTS_MSS       ? LWIP_TCP_OPT_LEN_MSS    : 0) + \
-  (flags & TF_SEG_OPTS_TS        ? LWIP_TCP_OPT_LEN_TS  : 0) + \
+  (flags & TF_SEG_OPTS_TS        ? LWIP_TCP_OPT_LEN_TS_OUT : 0) + \
-  (flags & TF_SEG_OPTS_WND_SCALE ? LWIP_TCP_OPT_LEN_WS  : 0)
+  (flags & TF_SEG_OPTS_WND_SCALE ? LWIP_TCP_OPT_LEN_WS_OUT : 0)
 
 /** This returns a TCP header option for MSS in an u32_t */
 #define TCP_BUILD_MSS_OPTION(mss) htonl(0x02040000 | ((mss) & 0xFFFF))