Re: [lwip-devel] [bug #51447] Sequence number comparisons invoke implementation-defined behavior

[Stian Skjelstad]

Looking at Hiromasa ITO numbers:

if (TCP_SEQ_BETWEEN(pcb->rcv_nxt, seqno + 1, seqno + tcplen - 1)) {

                      u32         u32         u32       u16

seqno + 1          : 0x13d897cb

seqno + tcplen - 1 : 0x13d897ca /* this is smaller than seqno + 1, so already here, we see that this test should currently ALWAYS fail, when viewed with human eyes. It should be impossible to fit a number within the given range*/

pcb->rcv_nxt       : 0x93d897ca

TCP_SEQ_GEQ(0x93d897ca, 0x13d897cb) && TCP_SEQ_LEQ (0x93d897ca, 0x13d897ca)

/* signed compare*/      /* signed compare*/

  (0x7FFFFFFF >= 0)   && ( 0x80000000 <= 0)

  (2147483647 >= 0)   && (-2147483648 <= 0)

It looks like we should inspect tcplen before checking TCP_SEQ_BETWEEN. Currently tcplen is checked for greater than zero, but it should be greater than 1 for the current test to be valid (parameter 3 should be same size or greater than 2)

Stian Skjelstad

On Tue, Jan 29, 2019 at 3:27 AM Hiromasa ITO <address@hidden> wrote:

Follow-up Comment #11, bug #51447 (project lwip):

I found some testcases caused a crash by this bug in fuzzing with AFL.
(I used experimental multi-packet fuzzing and some additional seeds made by
myself.)

It happened in tcp_recieve()...(around tcp_in.c:1429 in lwIP v2.1.2)


/* --- code snippet start --- */

if (TCP_SEQ_BETWEEN(pcb->rcv_nxt, seqno + 1, seqno + tcplen - 1)) {

    u32_t off32 = pcb->rcv_nxt - seqno;

    /* This assertion failed and crashed. */
    LWIP_ASSERT("insane offset!", (off32 < 0xffff));

    [...]

}

/* --- code snippet end --- */


In one testcase caused a crash, arguments of TCP_SEQ_BETWEEN were as below.


seqno + 1          : 0x13d897cb
seqno + tcplen - 1 : 0x13d897ca
pcb->rcv_nxt       : 0x93d897ca


In this case, TCP_SEQ_BETWEEN should return FALSE, but actually, returned TRUE
because ((pcb->rcv_nxt) - (seqno + tcplen - 1)) >= 2^31.
Then, off32 was greater than 0xffff, so LWIP_ASSERT failed and crashed.

I think,  this result shows that this bug is not just "theoretical" but
"practical".

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?51447>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/