From bf9951a11fddaef7a34e5e33fd6dcb82d8ff4ed2 Mon Sep 17 00:00:00 2001 From: Guillaume Hetier Date: Fri, 20 Mar 2026 12:33:57 -0700 Subject: [PATCH] Fix use-after-free in QuicRangeRemoveRange middle-split path In QuicRangeRemoveRange, the Sub pointer captured before QuicRangeMakeSpace becomes dangling when MakeSpace triggers QuicRangeGrow, which frees the old heap-allocated SubRanges array. The subsequent *NewSub = *Sub reads from freed memory. Fix: copy *Sub to a stack-local QUIC_SUBRANGE before calling MakeSpace, then assign the saved value to *NewSub. Add regression test RemoveRangeMiddleSplitWithGrow that fills 16 subranges (forcing heap allocation at AllocLength=16), then splits one via RemoveRange to trigger growth from 16 to 32. Fixes: #5824 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/core/range.c | 6 +++++- src/core/unittest/RangeTest.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/core/range.c b/src/core/range.c index 687e627b90..facbb0fc64 100644 --- a/src/core/range.c +++ b/src/core/range.c @@ -428,11 +428,15 @@ QuicRangeRemoveRange( // and the second part will be handled by the "left edge // overlaps" case. // + // Save Sub's value before MakeSpace, which may reallocate + // and free the old SubRanges array, invalidating Sub. + // + QUIC_SUBRANGE OldSub = *Sub; QUIC_SUBRANGE* NewSub = QuicRangeMakeSpace(Range, &i); if (NewSub == NULL) { return FALSE; } - *NewSub = *Sub; + *NewSub = OldSub; Sub = NewSub; } diff --git a/src/core/unittest/RangeTest.cpp b/src/core/unittest/RangeTest.cpp index 67bd0889a8..1b3be9832c 100644 --- a/src/core/unittest/RangeTest.cpp +++ b/src/core/unittest/RangeTest.cpp @@ -787,3 +787,33 @@ TEST(RangeTest, SearchRangeThree) ASSERT_EQ(index, 2); #endif } + +TEST(RangeTest, RemoveRangeMiddleSplitWithGrow) +{ + SmartRange range; + + // + // Fill to AllocLength=16 (heap-allocated) so a middle-split + // triggers Grow, which frees the old SubRanges array. + // + range.Add(100, 100); // [100..199] at index 0, will be split + for (uint32_t i = 1; i <= 15; i++) { + range.Add(1000 + i * 100); + } + ASSERT_EQ(range.ValidCount(), 16u); + + // + // Remove [140..159] from middle of [100..199]. Splits into + // [100..139] and [160..199], requiring 17 slots -> Grow. + // + ASSERT_TRUE(QuicRangeRemoveRange(&range.range, 140, 20)); + ASSERT_EQ(range.ValidCount(), 17u); + + auto sub0 = QuicRangeGet(&range.range, 0); + ASSERT_EQ(sub0->Low, 100ull); + ASSERT_EQ(sub0->Count, 40ull); + + auto sub1 = QuicRangeGet(&range.range, 1); + ASSERT_EQ(sub1->Low, 160ull); + ASSERT_EQ(sub1->Count, 40ull); +}