Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GH-16337: Use-after-free in SplHeap #16346

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 37 additions & 18 deletions ext/spl/spl_heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#define PTR_HEAP_BLOCK_SIZE 64

#define SPL_HEAP_CORRUPTED 0x00000001
#define SPL_HEAP_WRITE_LOCKED 0x00000002

zend_object_handlers spl_handler_SplHeap;
zend_object_handlers spl_handler_SplPriorityQueue;
Expand Down Expand Up @@ -278,12 +279,16 @@ static void spl_ptr_heap_insert(spl_ptr_heap *heap, void *elem, void *cmp_userda
heap->max_size *= 2;
}

heap->flags |= SPL_HEAP_WRITE_LOCKED;

/* sifting up */
for (i = heap->count; i > 0 && heap->cmp(spl_heap_elem(heap, (i-1)/2), elem, cmp_userdata) < 0; i = (i-1)/2) {
spl_heap_elem_copy(heap, spl_heap_elem(heap, i), spl_heap_elem(heap, (i-1)/2));
}
heap->count++;

heap->flags &= ~SPL_HEAP_WRITE_LOCKED;

if (EG(exception)) {
/* exception thrown during comparison */
heap->flags |= SPL_HEAP_CORRUPTED;
Expand Down Expand Up @@ -311,6 +316,8 @@ static int spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void *cmp_use
return FAILURE;
}

heap->flags |= SPL_HEAP_WRITE_LOCKED;

if (elem) {
spl_heap_elem_copy(heap, elem, spl_heap_elem(heap, 0));
} else {
Expand All @@ -334,6 +341,8 @@ static int spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void *cmp_use
}
}

heap->flags &= ~SPL_HEAP_WRITE_LOCKED;

if (EG(exception)) {
/* exception thrown during comparison */
heap->flags |= SPL_HEAP_CORRUPTED;
Expand Down Expand Up @@ -374,10 +383,14 @@ static spl_ptr_heap *spl_ptr_heap_clone(spl_ptr_heap *from) { /* {{{ */
static void spl_ptr_heap_destroy(spl_ptr_heap *heap) { /* {{{ */
int i;

heap->flags |= SPL_HEAP_WRITE_LOCKED;

for (i = 0; i < heap->count; ++i) {
heap->dtor(spl_heap_elem(heap, i));
}

heap->flags &= ~SPL_HEAP_WRITE_LOCKED;

efree(heap->elements);
efree(heap);
}
Expand Down Expand Up @@ -597,6 +610,21 @@ PHP_METHOD(SplHeap, isEmpty)
}
/* }}} */

static zend_result spl_heap_consistency_validations(const spl_heap_object *intern, bool write)
{
if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
return FAILURE;
}

if (write && (intern->heap->flags & SPL_HEAP_WRITE_LOCKED)) {
zend_throw_exception(spl_ce_RuntimeException, "Heap cannot be changed when it is already being modified.", 0);
return FAILURE;
}

return SUCCESS;
}

/* {{{ Push $value on the heap */
PHP_METHOD(SplHeap, insert)
{
Expand All @@ -609,8 +637,7 @@ PHP_METHOD(SplHeap, insert)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, true) != SUCCESS)) {
RETURN_THROWS();
}

Expand All @@ -632,8 +659,7 @@ PHP_METHOD(SplHeap, extract)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, true) != SUCCESS)) {
RETURN_THROWS();
}

Expand All @@ -658,8 +684,7 @@ PHP_METHOD(SplPriorityQueue, insert)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, true) != SUCCESS)) {
RETURN_THROWS();
}

Expand Down Expand Up @@ -699,8 +724,7 @@ PHP_METHOD(SplPriorityQueue, extract)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, true) != SUCCESS)) {
RETURN_THROWS();
}

Expand All @@ -726,8 +750,7 @@ PHP_METHOD(SplPriorityQueue, top)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, false) != SUCCESS)) {
RETURN_THROWS();
}

Expand Down Expand Up @@ -837,8 +860,7 @@ PHP_METHOD(SplHeap, top)

intern = Z_SPLHEAP_P(ZEND_THIS);

if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(intern, false) != SUCCESS)) {
RETURN_THROWS();
}

Expand Down Expand Up @@ -902,8 +924,7 @@ static zval *spl_heap_it_get_current_data(zend_object_iterator *iter) /* {{{ */
{
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);

if (object->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(object, false) != SUCCESS)) {
return NULL;
}

Expand All @@ -920,8 +941,7 @@ static zval *spl_pqueue_it_get_current_data(zend_object_iterator *iter) /* {{{ *
zend_user_iterator *user_it = (zend_user_iterator *) iter;
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);

if (object->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(object, false) != SUCCESS)) {
return NULL;
}

Expand Down Expand Up @@ -949,8 +969,7 @@ static void spl_heap_it_move_forward(zend_object_iterator *iter) /* {{{ */
{
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);

if (object->heap->flags & SPL_HEAP_CORRUPTED) {
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
if (UNEXPECTED(spl_heap_consistency_validations(object, false) != SUCCESS)) {
return;
}

Expand Down
49 changes: 49 additions & 0 deletions ext/spl/tests/gh16337.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
--TEST--
GH-16337 (Use-after-free in SplHeap)
--FILE--
<?php

class C {
function __toString() {
global $heap;
try {
$heap->extract();
} catch (Throwable $e) {
echo $e->getMessage(), "\n";
}
try {
$heap->insert(1);
} catch (Throwable $e) {
echo $e->getMessage(), "\n";
}
echo $heap->top(), "\n";
return "0";
}
}

$heap = new SplMinHeap;
for ($i = 0; $i < 100; $i++) {
$heap->insert((string) $i);
}
$heap->insert(new C);

?>
--EXPECT--
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Heap cannot be changed when it is already being modified.
Heap cannot be changed when it is already being modified.
0
Loading