[lwip-devel] bug in calling tcp_output_fill_options() ?

[Hannes Gredler]

hi,

I have run into a bug in tcp_output_fill_options() while trying to add TCP-AO 
(RFC5925) support.

--

it is my understanding that fourth argument in the callstack is the number of 
SACKs.

static void
tcp_output_fill_options(const struct tcp_pcb *pcb, struct pbuf *p, u8_t 
optflags, u8_t num_sacks)
                                                                                
  ^^^^^^^^^^^^^^^

however there is still 4 calls which pass in the total option length which gets 
then
misinterpreted as number of SACKs. which in my case caused a buffer overflow in
tcp_output_fill_options()

Q: shouldn't all those arguments reset to zero as per below patch ?

thanks,

/hannes


diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c
index 4e7cbb09..63c37448 100644
--- a/src/core/tcp_out.c
+++ b/src/core/tcp_out.c
@@ -2034,7 +2034,7 @@ tcp_rst(const struct tcp_pcb *pcb, u32_t seqno, u32_t 
ackno,
     LWIP_DEBUGF(TCP_DEBUG, ("tcp_rst: could not allocate memory for pbuf\n"));
     return;
   }
-  tcp_output_fill_options(pcb, p, 0, optlen);
+  tcp_output_fill_options(pcb, p, 0, 0);

 #ifdef LWIP_CUSTOM_HOOK_TCP_OUT_ADD
     LWIP_CUSTOM_HOOK_TCP_OUT_ADD(p, pcb);
@@ -2132,7 +2132,7 @@ tcp_keepalive(struct tcp_pcb *pcb)
                 ("tcp_keepalive: could not allocate memory for pbuf\n"));
     return ERR_MEM;
   }
-  tcp_output_fill_options(pcb, p, 0, optlen);
+  tcp_output_fill_options(pcb, p, 0, 0);
   err = tcp_output_control_segment(pcb, p, &pcb->local_ip, &pcb->remote_ip);

   LWIP_DEBUGF(TCP_DEBUG, ("tcp_keepalive: seqno %"U32_F" ackno %"U32_F" err 
%d.\n",
@@ -2214,7 +2214,7 @@ tcp_zero_window_probe(struct tcp_pcb *pcb)
   if (TCP_SEQ_LT(pcb->snd_nxt, snd_nxt)) {
     pcb->snd_nxt = snd_nxt;
   }
-  tcp_output_fill_options(pcb, p, 0, optlen);
+  tcp_output_fill_options(pcb, p, 0, 0);

   err = tcp_output_control_segment(pcb, p, &pcb->local_ip, &pcb->remote_ip);

@@ -2259,7 +2259,7 @@ tcp_custom_rst(s8_t *instName, u32_t seqno, u32_t ackno,
     LWIP_DEBUGF(TCP_DEBUG, ("tcp_rst: could not allocate memory for pbuf\n"));
     return;
   }
-  tcp_output_fill_options(pcb, p, 0, optlen);
+  tcp_output_fill_options(pcb, p, 0, 0);

   p->instName = instName;