Skip to content

Commit a56ff4f

Browse files
ndosschendossche
committed
Fix phpGH-16337: Use-after-free in SplHeap
We introduce a new flag to indicate when a heap or priority queue is write-locked. In principle we could've used SPL_HEAP_CORRUPTED too, but that won't be descriptive to users (and it's a lie too). Closes phpGH-16346.
1 parent 7cdd130 commit a56ff4f

File tree

3 files changed

+89
-18
lines changed

3 files changed

+89
-18
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ PHP NEWS
3232
. Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params).
3333
(nielsdos)
3434

35+
- SPL:
36+
. Fixed bug GH-16337 (Use-after-free in SplHeap). (nielsdos)
37+
3538
- XMLReader:
3639
. Fixed bug GH-16292 (Segmentation fault in ext/xmlreader/php_xmlreader.c).
3740
(nielsdos)

‎ext/spl/spl_heap.c‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#define PTR_HEAP_BLOCK_SIZE 64
3333

3434
#define SPL_HEAP_CORRUPTED 0x00000001
35+
#define SPL_HEAP_WRITE_LOCKED 0x00000002
3536

3637
zend_object_handlers spl_handler_SplHeap;
3738
zend_object_handlers spl_handler_SplPriorityQueue;
@@ -278,12 +279,16 @@ static void spl_ptr_heap_insert(spl_ptr_heap *heap, void *elem, void *cmp_userda
278279
heap->max_size *= 2;
279280
}
280281

282+
heap->flags |= SPL_HEAP_WRITE_LOCKED;
283+
281284
/* sifting up */
282285
for (i = heap->count; i > 0 && heap->cmp(spl_heap_elem(heap, (i-1)/2), elem, cmp_userdata) < 0; i = (i-1)/2) {
283286
spl_heap_elem_copy(heap, spl_heap_elem(heap, i), spl_heap_elem(heap, (i-1)/2));
284287
}
285288
heap->count++;
286289

290+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
291+
287292
if (EG(exception)) {
288293
/* exception thrown during comparison */
289294
heap->flags |= SPL_HEAP_CORRUPTED;
@@ -311,6 +316,8 @@ static int spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void *cmp_use
311316
return FAILURE;
312317
}
313318

319+
heap->flags |= SPL_HEAP_WRITE_LOCKED;
320+
314321
if (elem) {
315322
spl_heap_elem_copy(heap, elem, spl_heap_elem(heap, 0));
316323
} else {
@@ -334,6 +341,8 @@ static int spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void *cmp_use
334341
}
335342
}
336343

344+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
345+
337346
if (EG(exception)) {
338347
/* exception thrown during comparison */
339348
heap->flags |= SPL_HEAP_CORRUPTED;
@@ -374,10 +383,14 @@ static spl_ptr_heap *spl_ptr_heap_clone(spl_ptr_heap *from) { /* {{{ */
374383
static void spl_ptr_heap_destroy(spl_ptr_heap *heap) { /* {{{ */
375384
int i;
376385

386+
heap->flags |= SPL_HEAP_WRITE_LOCKED;
387+
377388
for (i = 0; i < heap->count; ++i) {
378389
heap->dtor(spl_heap_elem(heap, i));
379390
}
380391

392+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
393+
381394
efree(heap->elements);
382395
efree(heap);
383396
}
@@ -597,6 +610,21 @@ PHP_METHOD(SplHeap, isEmpty)
597610
}
598611
/* }}} */
599612

613+
static zend_result spl_heap_consistency_validations(const spl_heap_object *intern, bool write)
614+
{
615+
if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
616+
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
617+
return FAILURE;
618+
}
619+
620+
if (write && (intern->heap->flags & SPL_HEAP_WRITE_LOCKED)) {
621+
zend_throw_exception(spl_ce_RuntimeException, "Heap cannot be changed when it is already being modified.", 0);
622+
return FAILURE;
623+
}
624+
625+
return SUCCESS;
626+
}
627+
600628
/* {{{ Push $value on the heap */
601629
PHP_METHOD(SplHeap, insert)
602630
{
@@ -609,8 +637,7 @@ PHP_METHOD(SplHeap, insert)
609637

610638
intern = Z_SPLHEAP_P(ZEND_THIS);
611639

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

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

633660
intern = Z_SPLHEAP_P(ZEND_THIS);
634661

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

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

659685
intern = Z_SPLHEAP_P(ZEND_THIS);
660686

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

@@ -699,8 +724,7 @@ PHP_METHOD(SplPriorityQueue, extract)
699724

700725
intern = Z_SPLHEAP_P(ZEND_THIS);
701726

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

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

727751
intern = Z_SPLHEAP_P(ZEND_THIS);
728752

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

@@ -837,8 +860,7 @@ PHP_METHOD(SplHeap, top)
837860

838861
intern = Z_SPLHEAP_P(ZEND_THIS);
839862

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

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

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

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

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

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

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

‎ext/spl/tests/gh16337.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
GH-16337 (Use-after-free in SplHeap)
3+
--FILE--
4+
<?php
5+
6+
class C {
7+
function __toString() {
8+
global $heap;
9+
try {
10+
$heap->extract();
11+
} catch (Throwable $e) {
12+
echo $e->getMessage(), "\n";
13+
}
14+
try {
15+
$heap->insert(1);
16+
} catch (Throwable $e) {
17+
echo $e->getMessage(), "\n";
18+
}
19+
echo $heap->top(), "\n";
20+
return "0";
21+
}
22+
}
23+
24+
$heap = new SplMinHeap;
25+
for ($i = 0; $i < 100; $i++) {
26+
$heap->insert((string) $i);
27+
}
28+
$heap->insert(new C);
29+
30+
?>
31+
--EXPECT--
32+
Heap cannot be changed when it is already being modified.
33+
Heap cannot be changed when it is already being modified.
34+
0
35+
Heap cannot be changed when it is already being modified.
36+
Heap cannot be changed when it is already being modified.
37+
0
38+
Heap cannot be changed when it is already being modified.
39+
Heap cannot be changed when it is already being modified.
40+
0
41+
Heap cannot be changed when it is already being modified.
42+
Heap cannot be changed when it is already being modified.
43+
0
44+
Heap cannot be changed when it is already being modified.
45+
Heap cannot be changed when it is already being modified.
46+
0
47+
Heap cannot be changed when it is already being modified.
48+
Heap cannot be changed when it is already being modified.
49+
0

0 commit comments

Comments
 (0)