From 8499447741ad110d9e9ba00a1f5e377b6a883319 Mon Sep 17 00:00:00 2001 From: MarkedMuichiro Date: Sat, 21 Feb 2026 22:30:52 +0000 Subject: [PATCH 1/5] fix(forwarder): null-check allocations and fix resource leaks in quicforward - Check MsQuicStream allocs in ConnectionCallback; close raw stream handle on failure instead of leaking it - Check MsQuicConnection allocs in ListenerCallback; return QUIC_STATUS_OUT_OF_MEMORY so MsQuic refuses connection cleanly - Free SendContext before CXPLAT_FRE_ASSERT fires on unexpected send errors in StreamCallback --- src/tools/forwarder/forwarder.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/tools/forwarder/forwarder.cpp b/src/tools/forwarder/forwarder.cpp index 693a658edf..b5912d6c40 100644 --- a/src/tools/forwarder/forwarder.cpp +++ b/src/tools/forwarder/forwarder.cpp @@ -119,6 +119,10 @@ QUIC_STATUS StreamCallback( ForwardedSend::Delete(SendContext); return QUIC_STATUS_SUCCESS; } + // Any other failure is an unexpected invariant violation — free before fatal assert. + if (QUIC_FAILED(Status)) { + ForwardedSend::Delete(SendContext); + } CXPLAT_FRE_ASSERT(QUIC_SUCCEEDED(Status)); return BufferedMode ? QUIC_STATUS_SUCCESS : QUIC_STATUS_PENDING; } @@ -171,7 +175,16 @@ QUIC_STATUS ConnectionCallback( case QUIC_CONNECTION_EVENT_PEER_STREAM_STARTED: { //printf("c[%p] Peer stream started\n", Connection); auto PeerStream = new(std::nothrow) MsQuicStream(*PeerConn, Event->PEER_STREAM_STARTED.Flags, CleanUpAutoDelete, StreamCallback); + if (!PeerStream) { + MsQuic->StreamClose(Event->PEER_STREAM_STARTED.Stream); + return QUIC_STATUS_SUCCESS; + } auto LocalStream = new(std::nothrow) MsQuicStream(Event->PEER_STREAM_STARTED.Stream, CleanUpAutoDelete, StreamCallback, PeerStream); + if (!LocalStream) { + delete PeerStream; + MsQuic->StreamClose(Event->PEER_STREAM_STARTED.Stream); + return QUIC_STATUS_SUCCESS; + } PeerStream->Context = LocalStream; //printf("s[%p] Started -> [%p]\n", LocalStream, PeerStream); break; @@ -194,7 +207,14 @@ QUIC_STATUS ListenerCallback( { if (Event->Type == QUIC_LISTENER_EVENT_NEW_CONNECTION) { auto BackEndConn = new(std::nothrow) MsQuicConnection(*Registration, CleanUpAutoDelete, ConnectionCallback); + if (!BackEndConn) { + return QUIC_STATUS_OUT_OF_MEMORY; + } auto FrontEndConn = new(std::nothrow) MsQuicConnection(Event->NEW_CONNECTION.Connection, CleanUpAutoDelete, ConnectionCallback, BackEndConn); + if (!FrontEndConn) { + delete BackEndConn; + return QUIC_STATUS_OUT_OF_MEMORY; + } BackEndConn->Context = FrontEndConn; //printf("c[%p] Created -> [%p]\n", FrontEndConn, BackEndConn); CXPLAT_FRE_ASSERT(QUIC_SUCCEEDED(BackEndConn->Start(*BackEndConfiguration, BackEndTarget, BackEndPort))); From bbd77e9b87b4fe63a3f85e22a727d83da09cafbe Mon Sep 17 00:00:00 2001 From: MarkedMuichiro Date: Mon, 23 Feb 2026 20:16:15 +0000 Subject: [PATCH 2/5] fix(forwarder): remove unreachable cleanup before assert in StreamCallback --- src/tools/forwarder/forwarder.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tools/forwarder/forwarder.cpp b/src/tools/forwarder/forwarder.cpp index b5912d6c40..6c05582f68 100644 --- a/src/tools/forwarder/forwarder.cpp +++ b/src/tools/forwarder/forwarder.cpp @@ -119,10 +119,6 @@ QUIC_STATUS StreamCallback( ForwardedSend::Delete(SendContext); return QUIC_STATUS_SUCCESS; } - // Any other failure is an unexpected invariant violation — free before fatal assert. - if (QUIC_FAILED(Status)) { - ForwardedSend::Delete(SendContext); - } CXPLAT_FRE_ASSERT(QUIC_SUCCEEDED(Status)); return BufferedMode ? QUIC_STATUS_SUCCESS : QUIC_STATUS_PENDING; } From e9b8e891f31209e478fae098eccc2b3f34b4ab86 Mon Sep 17 00:00:00 2001 From: MarkedMuichiro Date: Tue, 24 Feb 2026 03:14:52 +0000 Subject: [PATCH 3/5] fix(forwarder): replace delete with Close() and add IsValid() checks for CleanUpAutoDelete objects --- src/tools/forwarder/forwarder.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/tools/forwarder/forwarder.cpp b/src/tools/forwarder/forwarder.cpp index 6c05582f68..79bc669197 100644 --- a/src/tools/forwarder/forwarder.cpp +++ b/src/tools/forwarder/forwarder.cpp @@ -171,14 +171,15 @@ QUIC_STATUS ConnectionCallback( case QUIC_CONNECTION_EVENT_PEER_STREAM_STARTED: { //printf("c[%p] Peer stream started\n", Connection); auto PeerStream = new(std::nothrow) MsQuicStream(*PeerConn, Event->PEER_STREAM_STARTED.Flags, CleanUpAutoDelete, StreamCallback); - if (!PeerStream) { + if (!PeerStream || !PeerStream->IsValid()) { + if (PeerStream) PeerStream->Close(); MsQuic->StreamClose(Event->PEER_STREAM_STARTED.Stream); return QUIC_STATUS_SUCCESS; } auto LocalStream = new(std::nothrow) MsQuicStream(Event->PEER_STREAM_STARTED.Stream, CleanUpAutoDelete, StreamCallback, PeerStream); if (!LocalStream) { - delete PeerStream; MsQuic->StreamClose(Event->PEER_STREAM_STARTED.Stream); + PeerStream->Close(); return QUIC_STATUS_SUCCESS; } PeerStream->Context = LocalStream; @@ -203,12 +204,13 @@ QUIC_STATUS ListenerCallback( { if (Event->Type == QUIC_LISTENER_EVENT_NEW_CONNECTION) { auto BackEndConn = new(std::nothrow) MsQuicConnection(*Registration, CleanUpAutoDelete, ConnectionCallback); - if (!BackEndConn) { + if (!BackEndConn || !BackEndConn->IsValid()) { + if (BackEndConn) BackEndConn->Close(); return QUIC_STATUS_OUT_OF_MEMORY; } auto FrontEndConn = new(std::nothrow) MsQuicConnection(Event->NEW_CONNECTION.Connection, CleanUpAutoDelete, ConnectionCallback, BackEndConn); if (!FrontEndConn) { - delete BackEndConn; + BackEndConn->Close(); return QUIC_STATUS_OUT_OF_MEMORY; } BackEndConn->Context = FrontEndConn; From 6822887c20bfeea5038cf85aa7509cd49e1ed535 Mon Sep 17 00:00:00 2001 From: MarkedMuichiro Date: Mon, 2 Mar 2026 14:19:30 -0800 Subject: [PATCH 4/5] Update src/tools/forwarder/forwarder.cpp Add braces Co-authored-by: Guillaume Hetier --- src/tools/forwarder/forwarder.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/forwarder/forwarder.cpp b/src/tools/forwarder/forwarder.cpp index 79bc669197..dbac09836f 100644 --- a/src/tools/forwarder/forwarder.cpp +++ b/src/tools/forwarder/forwarder.cpp @@ -172,7 +172,9 @@ QUIC_STATUS ConnectionCallback( //printf("c[%p] Peer stream started\n", Connection); auto PeerStream = new(std::nothrow) MsQuicStream(*PeerConn, Event->PEER_STREAM_STARTED.Flags, CleanUpAutoDelete, StreamCallback); if (!PeerStream || !PeerStream->IsValid()) { - if (PeerStream) PeerStream->Close(); + if (PeerStream) { + PeerStream->Close(); + } MsQuic->StreamClose(Event->PEER_STREAM_STARTED.Stream); return QUIC_STATUS_SUCCESS; } From 4ced9c5ee950ba0a960465f050fba13bd8cdd344 Mon Sep 17 00:00:00 2001 From: MarkedMuichiro Date: Mon, 2 Mar 2026 15:09:45 -0800 Subject: [PATCH 5/5] Add curly braces for style consistency --- src/tools/forwarder/forwarder.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/forwarder/forwarder.cpp b/src/tools/forwarder/forwarder.cpp index dbac09836f..4401263e5b 100644 --- a/src/tools/forwarder/forwarder.cpp +++ b/src/tools/forwarder/forwarder.cpp @@ -207,7 +207,9 @@ QUIC_STATUS ListenerCallback( if (Event->Type == QUIC_LISTENER_EVENT_NEW_CONNECTION) { auto BackEndConn = new(std::nothrow) MsQuicConnection(*Registration, CleanUpAutoDelete, ConnectionCallback); if (!BackEndConn || !BackEndConn->IsValid()) { - if (BackEndConn) BackEndConn->Close(); + if (BackEndConn) { + BackEndConn->Close(); + } return QUIC_STATUS_OUT_OF_MEMORY; } auto FrontEndConn = new(std::nothrow) MsQuicConnection(Event->NEW_CONNECTION.Connection, CleanUpAutoDelete, ConnectionCallback, BackEndConn);