buglwIP - A Lightweight TCP/IP stack - Bugs: bug #57433, use-after-free caused by...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #57433: use-after-free caused by tcp_input_delayed_close

Submitter:  Gar User <ghessler>
Submitted:  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 Planned Release:  None
lwIP version:  2.1.0

Tue 17 Dec 2019 07:49:31 PM UTC, comment #1: 

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.

Gar User <ghessler>
Tue 17 Dec 2019 02:12:23 PM UTC, original submission:  

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.

Gar User <ghessler>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ghessler (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    No changes have been made to this item

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code