From d9555bea1b65fe2ea7e3815486fe8f6c7c3a38dd Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 7 May 2021 12:02:00 +0200 Subject: [PATCH] api: discourage UaF on teardown While it does make sense to make readers unblock by setting the read event on teardown, this is something that consumers of the library should do _before_ calling EndSession, not something that makes sense for the library to do itself. The reason is that, in the hypothetical case in which this makes sense, immediately after unblocking the reader via SetEvent, the function goes on to free all of the memory that that reader might want to use. So, rather, the proper shutdown flow is from the application side, and looks like: Closing = true; SetEvent(WintunGetReadWaitEvent()); WaitForReadersToReturn(); WintunEndSession(); Alternatively, rather than using WaitForSingleObject on the read event, consumers can WaitForMultipleObjects and include a shutdown event, which is what the example code does. Signed-off-by: Jason A. Donenfeld --- api/session.c | 1 - 1 file changed, 1 deletion(-) diff --git a/api/session.c b/api/session.c index ef65214..763dd83 100644 --- a/api/session.c +++ b/api/session.c @@ -154,7 +154,6 @@ out: void WINAPI WintunEndSession(_In_ TUN_SESSION *Session) { - SetEvent(Session->Descriptor.Send.TailMoved); // Wake the reader if it's sleeping. DeleteCriticalSection(&Session->Send.Lock); DeleteCriticalSection(&Session->Receive.Lock); CloseHandle(Session->Handle);