[bug #57433] use-after-free caused by tcp_input_delayed_close

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #57433] use-after-free caused by tcp_input_delayed_close

Simon Goldschmidt
URL:
  <https://savannah.nongnu.org/bugs/?57433>

                 Summary: use-after-free caused by tcp_input_delayed_close
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: ghessler
            Submitted on: Tue 17 Dec 2019 02:12:23 PM UTC
                Category: TCP
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: 2.1.0

    _______________________________________________________

Details:

This topic was originally posted on lwip-devel list on Jan 24, 2019.  It
appears to have never been resolved.
I am also seeing this same problem when running v2.1.0.


I'm running a tcp server using LWIP and upon termination of the connection,
both sides do a shutdown.

The bug occurs in form of a race condition:
- the lwip server calls SHUT_WR
- the client calls SHUT_WR, once lwip saw this, it sets TF_RXCLOSED in
"pcb->flags"
- the client closes the connection, lwip sees this, adds TF_CLOSED to
recv_flags, and then deletes the pcb within "tcp_input_delayed_close".

The problem here is that "tcp_input_delayed_close" only calls the "pcb->errf"
callback on this condition:
"if (!(pcb->flags & TF_RXCLOSED))"

I don't really know why that was done in first place, but because of this, the
pcb gets freed without notifying the user(which would set conn->pcb.tcp to
NULL) in case the RX side was closed already.

On the next call to shutdown or close, this results in use-after-free (double
free of the pcb).


Also it looks like the code in question was added in 2012 and has never been
changed that much.
To be more specific, it got introduced in this commit:
http://git.savannah.nongnu.org/cgit/lwip.git/commit/?id=13075460ea10c2902ea2055d18bbcfa73cec8523
and the questionable condition is in the following line today:
http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/core/tcp_in.c?id=master#n607

The problem does seem to go away if I just unconditionally call TCP_EVENT_ERR,
but I'm not sure if that is the correct solution or if this would have
unwanted side effects.





    _______________________________________________________

Reply to this item at:

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

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


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #57433] use-after-free caused by tcp_input_delayed_close

Simon Goldschmidt
Follow-up Comment #1, bug #57433 (project lwip):

The original poster commented that the problem was solved by removing the
condition, and unconditionally calling TCP_EVENT_ERR in
tcp_input_delayed_close().

I have seen the same result when running v2.1.0.

This may be the fix, but I share the same concerns with the original poster
regarding any side-effects.

    _______________________________________________________

Reply to this item at:

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

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


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel