fixed bug #45559: Window scaling casts u32_t to u16_t without checks

CHANGELOG

@@ -249,10 +249,13 @@ HISTORY
 
   ++ Bugfixes:
 
+  2015-08-28: Simon Goldschmidt
+  * tcp: fixed bug #45559: Window scaling casts u32_t to u16_t without checks
+
   2015-08-26: Simon Goldschmidt
   * ip6_frag.h/.c: fixed bug bug #41009: IPv6 reassembly broken on 64-bit platforms:
     define IPV6_FRAG_COPYHEADER==1 on these platforms to copy the IPv6 header
-	instead of referencing it, which gives more room for struct ip6_reass_helper
+    instead of referencing it, which gives more room for struct ip6_reass_helper
 
   2015-08-25: Simon Goldschmidt
   * sockets.c: fixed bug #45827: recvfrom: TCP window is updated with MSG_PEEK

src/core/tcp.c

@@ -175,7 +175,7 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
   err_t err;
 
   if (rst_on_unacked_data && ((pcb->state == ESTABLISHED) || (pcb->state == CLOSE_WAIT))) {
-    if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND)) {
+    if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND_MAX(pcb))) {
       /* Not all data received by application, send RST to tell the remote
          side about this. */
       LWIP_ASSERT("pcb->flags & TF_RXCLOSED", pcb->flags & TF_RXCLOSED);
@@ -673,15 +673,15 @@ tcp_recved(struct tcp_pcb *pcb, u16_t len)
     pcb->state != LISTEN);
 
   pcb->rcv_wnd += len;
-  if (pcb->rcv_wnd > TCP_WND) {
-    pcb->rcv_wnd = TCP_WND;
+  if (pcb->rcv_wnd > TCP_WND_MAX(pcb)) {
+    pcb->rcv_wnd = TCP_WND_MAX(pcb);
   } else if(pcb->rcv_wnd == 0) {
     /* rcv_wnd overflowed */
     if ((pcb->state == CLOSE_WAIT) || (pcb->state == LAST_ACK)) {
       /* In passive close, we allow this, since the FIN bit is added to rcv_wnd
          by the stack itself, since it is not mandatory for an application
          to call tcp_recved() for the FIN bit, but e.g. the netconn API does so. */
-      pcb->rcv_wnd = TCP_WND;
+      pcb->rcv_wnd = TCP_WND_MAX(pcb);
     } else {
       LWIP_ASSERT("tcp_recved: len wrapped rcv_wnd\n", 0);
     }
@@ -699,7 +699,7 @@ tcp_recved(struct tcp_pcb *pcb, u16_t len)
   }
 
   LWIP_DEBUGF(TCP_DEBUG, ("tcp_recved: received %"U16_F" bytes, wnd %"U16_F" (%"U16_F").\n",
-         len, pcb->rcv_wnd, TCP_WND - pcb->rcv_wnd));
+         len, pcb->rcv_wnd, TCP_WND_MAX(pcb) - pcb->rcv_wnd));
 }
 
 /**
@@ -814,8 +814,9 @@ tcp_connect(struct tcp_pcb *pcb, const ip_addr_t *ipaddr, u16_t port,
   pcb->snd_nxt = iss;
   pcb->lastack = iss - 1;
   pcb->snd_lbb = iss - 1;
-  pcb->rcv_wnd = TCP_WND;
-  pcb->rcv_ann_wnd = TCP_WND;
+  /* Start with a window that does not need scaling. When window scaling is
+     enabled and used, the window is enlarged when both sides agree on scaling. */
+  pcb->rcv_wnd = pcb->rcv_ann_wnd = TCPWND_MIN16(TCP_WND);
   pcb->rcv_ann_right_edge = pcb->rcv_nxt;
   pcb->snd_wnd = TCP_WND;
   /* As initial send MSS, we use TCP_MSS but limit it to 536.
@@ -1204,7 +1205,7 @@ tcp_process_refused_data(struct tcp_pcb *pcb)
          ) {
         /* correct rcv_wnd as the application won't call tcp_recved()
            for the FIN's seqno */
-        if (pcb->rcv_wnd != TCP_WND) {
+        if (pcb->rcv_wnd != TCP_WND_MAX(pcb)) {
           pcb->rcv_wnd++;
         }
         TCP_EVENT_CLOSED(pcb, err);
@@ -1474,8 +1475,9 @@ tcp_alloc(u8_t prio)
     pcb->prio = prio;
     pcb->snd_buf = TCP_SND_BUF;
     pcb->snd_queuelen = 0;
-    pcb->rcv_wnd = TCP_WND;
-    pcb->rcv_ann_wnd = TCP_WND;
+    /* Start with a window that does not need scaling. When window scaling is
+       enabled and used, the window is enlarged when both sides agree on scaling. */
+    pcb->rcv_wnd = pcb->rcv_ann_wnd = TCPWND_MIN16(TCP_WND);
 #if LWIP_WND_SCALE
     /* snd_scale and rcv_scale are zero unless both sides agree to use scaling */
     pcb->snd_scale = 0;

src/core/tcp_in.c

@@ -460,7 +460,7 @@ tcp_input(struct pbuf *p, struct netif *inp)
           } else {
             /* correct rcv_wnd as the application won't call tcp_recved()
                for the FIN's seqno */
-            if (pcb->rcv_wnd != TCP_WND) {
+            if (pcb->rcv_wnd != TCP_WND_MAX(pcb)) {
               pcb->rcv_wnd++;
             }
             TCP_EVENT_CLOSED(pcb, err);
@@ -629,14 +629,14 @@ tcp_timewait_input(struct tcp_pcb *pcb)
    * - first check sequence number - we skip that one in TIME_WAIT (always
    *   acceptable since we only send ACKs)
    * - second check the RST bit (... return) */
-  if (flags & TCP_RST)  {
+  if (flags & TCP_RST) {
     return ERR_OK;
   }
   /* - fourth, check the SYN bit, */
   if (flags & TCP_SYN) {
     /* If an incoming segment is not acceptable, an acknowledgment
        should be sent in reply */
-    if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt+pcb->rcv_wnd)) {
+    if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd)) {
       /* If the SYN is in the window it is an error, send a reset */
       tcp_rst(ackno, seqno + tcplen, ip_current_dest_addr(),
         ip_current_src_addr(), tcphdr->dest, tcphdr->src);
@@ -648,7 +648,7 @@ tcp_timewait_input(struct tcp_pcb *pcb)
     pcb->tmr = tcp_ticks;
   }
 
-  if ((tcplen > 0))  {
+  if ((tcplen > 0)) {
     /* Acknowledge data, FIN or out-of-window SYN */
     pcb->flags |= TF_ACK_NOW;
     return tcp_output(pcb);
@@ -684,8 +684,8 @@ tcp_process(struct tcp_pcb *pcb)
         acceptable = 1;
       }
     } else {
-      if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, 
-                          pcb->rcv_nxt+pcb->rcv_wnd)) {
+      if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt,
+                          pcb->rcv_nxt + pcb->rcv_wnd)) {
         acceptable = 1;
       }
     }
@@ -1310,7 +1310,7 @@ tcp_receive(struct tcp_pcb *pcb)
       inseg.tcphdr->seqno = seqno = pcb->rcv_nxt;
     }
     else {
-      if (TCP_SEQ_LT(seqno, pcb->rcv_nxt)){
+      if (TCP_SEQ_LT(seqno, pcb->rcv_nxt)) {
         /* the whole segment is < rcv_nxt */
         /* must be a duplicate of a packet that has already been correctly handled */
 
@@ -1322,7 +1322,7 @@ tcp_receive(struct tcp_pcb *pcb)
     /* The sequence number must be within the window (above rcv_nxt
        and below rcv_nxt + rcv_wnd) in order to be further
        processed. */
-    if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, 
+    if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt,
                         pcb->rcv_nxt + pcb->rcv_wnd - 1)){
       if (pcb->rcv_nxt == seqno) {
         /* The incoming segment is the next in sequence. We check if
@@ -1331,17 +1331,18 @@ tcp_receive(struct tcp_pcb *pcb)
         tcplen = TCP_TCPLEN(&inseg);
 
         if (tcplen > pcb->rcv_wnd) {
-          LWIP_DEBUGF(TCP_INPUT_DEBUG, 
+          LWIP_DEBUGF(TCP_INPUT_DEBUG,
                       ("tcp_receive: other end overran receive window"
                        "seqno %"U32_F" len %"U16_F" right edge %"U32_F"\n",
                        seqno, tcplen, pcb->rcv_nxt + pcb->rcv_wnd));
           if (TCPH_FLAGS(inseg.tcphdr) & TCP_FIN) {
-            /* Must remove the FIN from the header as we're trimming 
+            /* Must remove the FIN from the header as we're trimming
              * that byte of sequence-space from the packet */
             TCPH_FLAGS_SET(inseg.tcphdr, TCPH_FLAGS(inseg.tcphdr) & ~(unsigned int)TCP_FIN);
           }
           /* Adjust length of segment to fit in the window. */
-          inseg.len = pcb->rcv_wnd;
+          TCPWND_CHECK16(pcb->rcv_wnd);
+          inseg.len = (u16_t)pcb->rcv_wnd;
           if (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) {
             inseg.len -= 1;
           }
@@ -1643,9 +1644,7 @@ tcp_receive(struct tcp_pcb *pcb)
   } else {
     /* Segments with length 0 is taken care of here. Segments that
        fall out of the window are ACKed. */
-    /*if (TCP_SEQ_GT(pcb->rcv_nxt, seqno) ||
-      TCP_SEQ_GEQ(seqno, pcb->rcv_nxt + pcb->rcv_wnd)) {*/
-    if(!TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd-1)){
+    if(!TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd - 1)){
       tcp_ack_now(pcb);
     }
   }
@@ -1725,6 +1724,10 @@ tcp_parseopt(struct tcp_pcb *pcb)
           }
           pcb->rcv_scale = TCP_RCV_SCALE;
           pcb->flags |= TF_WND_SCALE;
+          /* window scaling is enabled, we can use the full receive window */
+          LWIP_ASSERT("window not at default value", pcb->rcv_wnd == TCPWND_MIN16(TCP_WND));
+          LWIP_ASSERT("window not at default value", pcb->rcv_ann_wnd == TCPWND_MIN16(TCP_WND));
+          pcb->rcv_wnd = pcb->rcv_ann_wnd = TCP_WND;
         }
         break;
 #endif

src/core/tcp_out.c

@@ -121,7 +121,7 @@ tcp_output_alloc_header(struct tcp_pcb *pcb, u16_t optlen, u16_t datalen,
     tcphdr->seqno = seqno_be;
     tcphdr->ackno = htonl(pcb->rcv_nxt);
     TCPH_HDRLEN_FLAGS_SET(tcphdr, (5 + optlen / 4), TCP_ACK);
-    tcphdr->wnd = htons(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd));
+    tcphdr->wnd = htons(TCPWND_MIN16(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd)));
     tcphdr->chksum = 0;
     tcphdr->urgp = 0;
 
@@ -387,7 +387,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags)
 #endif /* TCP_CHECKSUM_ON_COPY */
   err_t err;
   /* don't allocate segments bigger than half the maximum window we ever received */
-  u16_t mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2);
+  u16_t mss_local = LWIP_MIN(pcb->mss, TCPWND_MIN16(pcb->snd_wnd_max/2));
   mss_local = mss_local ? mss_local : pcb->mss;
 
 #if LWIP_NETIF_TX_SINGLE_PBUF
@@ -1141,11 +1141,11 @@ tcp_output_segment(struct tcp_seg *seg, struct tcp_pcb *pcb)
   if (seg->flags & TF_SEG_OPTS_WND_SCALE) {
     /* The Window field in a SYN segment itself (the only type where we send
        the window scale option) is never scaled. */
-    seg->tcphdr->wnd = htons(pcb->rcv_ann_wnd);
+    seg->tcphdr->wnd = htons(TCPWND_MIN16(pcb->rcv_ann_wnd));
   } else
 #endif /* LWIP_WND_SCALE */
   {
-    seg->tcphdr->wnd = htons(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd));
+    seg->tcphdr->wnd = htons(TCPWND_MIN16(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd)));
   }
 
   pcb->rcv_ann_right_edge = pcb->rcv_nxt + pcb->rcv_ann_wnd;

src/include/lwip/opt.h

@@ -1245,7 +1245,7 @@
  * explicit window update
  */
 #ifndef TCP_WND_UPDATE_THRESHOLD
-#define TCP_WND_UPDATE_THRESHOLD   (TCP_WND / 4)
+#define TCP_WND_UPDATE_THRESHOLD   LWIP_MIN((TCP_WND / 4), (TCP_MSS * 4))
 #endif
 
 /**

src/include/lwip/tcp.h

@@ -128,13 +128,15 @@ typedef err_t (*tcp_connected_fn)(void *arg, struct tcp_pcb *tpcb, err_t err);
 #if LWIP_WND_SCALE
 #define RCV_WND_SCALE(pcb, wnd) (((wnd) >> (pcb)->rcv_scale))
 #define SND_WND_SCALE(pcb, wnd) (((wnd) << (pcb)->snd_scale))
-#define TCPWND16(x) ((u16_t)LWIP_MIN((x), 0xFFFF))
+#define TCPWND16(x)             ((u16_t)LWIP_MIN((x), 0xFFFF))
+#define TCP_WND_MAX(pcb)        ((tcpwnd_size_t)(((pcb)->flags & TF_WND_SCALE) ? TCP_WND : TCPWND16(TCP_WND)))
 typedef u32_t tcpwnd_size_t;
 typedef u16_t tcpflags_t;
 #else
 #define RCV_WND_SCALE(pcb, wnd) (wnd)
 #define SND_WND_SCALE(pcb, wnd) (wnd)
-#define TCPWND16(x) (x)
+#define TCPWND16(x)             (x)
+#define TCP_WND_MAX(pcb)        TCP_WND
 typedef u16_t tcpwnd_size_t;
 typedef u8_t tcpflags_t;
 #endif

src/include/lwip/tcp_impl.h

@@ -332,11 +332,15 @@ struct tcp_seg {
 #define TCP_BUILD_MSS_OPTION(mss) htonl(0x02040000 | ((mss) & 0xFFFF))
 
 #if LWIP_WND_SCALE
-#define TCPWNDSIZE_F  U32_F
-#define TCPWND_MAX    0xFFFFFFFFU
+#define TCPWNDSIZE_F       U32_F
+#define TCPWND_MAX         0xFFFFFFFFU
+#define TCPWND_CHECK16(x)  LWIP_ASSERT("window size > 0xFFFF", (x) <= 0xFFFF)
+#define TCPWND_MIN16(x)    ((u16_t)LWIP_MIN((x), 0xFFFF))
 #else /* LWIP_WND_SCALE */
-#define TCPWNDSIZE_F  U16_F
-#define TCPWND_MAX    0xFFFFU
+#define TCPWNDSIZE_F       U16_F
+#define TCPWND_MAX         0xFFFFU
+#define TCPWND_CHECK16(x)
+#define TCPWND_MIN16(x)    x
 #endif /* LWIP_WND_SCALE */
 
 /* Global variables: */