Skip to content

CFBinaryHeap: fix out-of-bounds accesses in copy and remove#48

Open
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix/cfbinaryheap-oob
Open

CFBinaryHeap: fix out-of-bounds accesses in copy and remove#48
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix/cfbinaryheap-oob

Conversation

@DTW-Thalion

Copy link
Copy Markdown
Contributor

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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant