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

[address@hidden]

Am 01.04.2020 um 11:05 schrieb 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 ?

Seems like you're right, yes.

Regards,
Simon

>
> 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;
>
> _______________________________________________
> lwip-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/lwip-devel
>