From 342863a0a81f15dc2093acb0e2922c95ab080f36 Mon Sep 17 00:00:00 2001 From: Todd White Date: Sun, 28 Jun 2026 09:01:18 -0400 Subject: [PATCH] CFBinaryHeap: fix out-of-bounds accesses in copy and remove Two out-of-bounds heap accesses, both reachable from the public API and confirmed with AddressSanitizer: * CFBinaryHeapCreateCopy() created the new heap with the requested capacity and then memcpy'd _count values into it. When the requested capacity was smaller than the source count (e.g. 0) the destination was only DEFAULT_HEAP_CAPACITY (15) slots, so copying a larger heap overran it (heap-buffer-overflow WRITE in CFBinaryHeapCreateCopy). * CFBinaryHeapRemoveMinimumValue() unconditionally read _values[0] and _values[_count - 1] and decremented _count. On an empty heap that read _values[-1] (heap-buffer-overflow READ) and left _count at -1. Enlarge the copy's capacity to at least the source count, and make removing from an empty heap a no-op. Adds regression tests. --- ChangeLog | 7 +++++++ Source/CFBinaryHeap.c | 9 ++++++++- Tests/CFBinaryHeap/general.m | 29 ++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0604f3de..3aa77b5e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2026-06-28 Todd White + * Source/CFBinaryHeap.c (CFBinaryHeapCreateCopy): Enlarge the copy's + capacity to at least the source count before copying the values. + (CFBinaryHeapRemoveMinimumValue): Do nothing on an empty heap + instead of reading _values[-1] and setting _count to -1. + * Tests/CFBinaryHeap/general.m: Regression tests. + 2021-09-30 Frederik Seiffert * Source/CFDate.c: Fix logic in CFGregorianDateIsValid() diff --git a/Source/CFBinaryHeap.c b/Source/CFBinaryHeap.c index 2ab01998..46f68f26 100644 --- a/Source/CFBinaryHeap.c +++ b/Source/CFBinaryHeap.c @@ -206,7 +206,11 @@ CFBinaryHeapCreateCopy (CFAllocatorRef alloc, CFIndex capacity, CFBinaryHeapRef heap) { CFBinaryHeapRef ret; - + + /* The copy must be able to hold every value regardless of the requested + capacity. */ + if (capacity < heap->_count) + capacity = heap->_count; ret = CFBinaryHeapCreate (alloc, capacity, heap->_callBacks, &heap->_context); memcpy (ret->_values, heap->_values, sizeof(void*) * heap->_count); @@ -369,6 +373,9 @@ CFBinaryHeapRemoveMinimumValue (CFBinaryHeapRef heap) const void *last; void *info; + if (heap->_count == 0) + return; + release = heap->_callBacks->release; if (release) release (CFGetAllocator(heap), heap->_values[0]); diff --git a/Tests/CFBinaryHeap/general.m b/Tests/CFBinaryHeap/general.m index 7b945ca7..0388d914 100644 --- a/Tests/CFBinaryHeap/general.m +++ b/Tests/CFBinaryHeap/general.m @@ -35,6 +35,33 @@ int main (void) PASS_CF(n == 5, "Minimum value in binary heap is %d.", (int)n); CFRelease (heap); - + + /* A copy with a requested capacity smaller than the count must still + hold every value (no write past the buffer). */ + { + CFBinaryHeapRef big = CFBinaryHeapCreate (NULL, 0, NULL, NULL); + CFBinaryHeapRef copy; + CFIndex i; + + for (i = 0; i < 20; i++) + CFBinaryHeapAddValue (big, (const void *)(i + 1)); + copy = CFBinaryHeapCreateCopy (NULL, 0, big); + PASS_CF(CFBinaryHeapGetCount (copy) == 20 + && (CFIndex)CFBinaryHeapGetMinimum (copy) == 1, + "A copy with capacity 0 holds all 20 values."); + CFRelease (copy); + CFRelease (big); + } + + /* Removing from an empty heap must be a no-op, not read out of bounds. */ + { + CFBinaryHeapRef empty = CFBinaryHeapCreate (NULL, 0, NULL, NULL); + + CFBinaryHeapRemoveMinimumValue (empty); + PASS_CF(CFBinaryHeapGetCount (empty) == 0, + "Removing from an empty heap leaves it empty."); + CFRelease (empty); + } + return 0; } \ No newline at end of file