Skip to content

Commit 105cf92

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.4'
* PHP-8.4: Add missing hierarchy checks to replaceChild Fix GH-16337: Use-after-free in SplHeap
2 parents baa76be + c31eac7 commit 105cf92

File tree

4 files changed

+118
-34
lines changed

4 files changed

+118
-34
lines changed

‎ext/dom/node.c

+5-16
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ static xmlNodePtr dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlN
810810
}
811811
/* }}} */
812812

813-
static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNodePtr child, bool stricterror)
813+
static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNodePtr child, bool stricterror, bool warn_empty_fragment)
814814
{
815815
if (dom_node_is_read_only(parentp) == SUCCESS ||
816816
(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
@@ -828,7 +828,7 @@ static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNode
828828
return false;
829829
}
830830

831-
if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
831+
if (warn_empty_fragment && child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
832832
/* TODO Drop Warning? */
833833
php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
834834
return false;
@@ -855,7 +855,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
855855
xmlNodePtr new_child = NULL;
856856
bool stricterror = dom_get_strict_error(intern->document);
857857

858-
if (!dom_node_check_legacy_insertion_validity(parentp, child, stricterror)) {
858+
if (!dom_node_check_legacy_insertion_validity(parentp, child, stricterror, true)) {
859859
RETURN_FALSE;
860860
}
861861

@@ -1152,18 +1152,7 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern)
11521152
RETURN_FALSE;
11531153
}
11541154

1155-
if (!nodep->children) {
1156-
RETURN_FALSE;
1157-
}
1158-
1159-
if (dom_node_is_read_only(nodep) == SUCCESS ||
1160-
(newchild->parent != NULL && dom_node_is_read_only(newchild->parent) == SUCCESS)) {
1161-
php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
1162-
RETURN_FALSE;
1163-
}
1164-
1165-
if (dom_hierarchy(nodep, newchild) == FAILURE) {
1166-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
1155+
if (!dom_node_check_legacy_insertion_validity(nodep, newchild, stricterror, false)) {
11671156
RETURN_FALSE;
11681157
}
11691158

@@ -1277,7 +1266,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern,
12771266

12781267
bool stricterror = dom_get_strict_error(intern->document);
12791268

1280-
if (!dom_node_check_legacy_insertion_validity(nodep, child, stricterror)) {
1269+
if (!dom_node_check_legacy_insertion_validity(nodep, child, stricterror, true)) {
12811270
RETURN_FALSE;
12821271
}
12831272

‎ext/dom/tests/replaceChild_attribute_validation.phpt

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
replaceChild with attribute children
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$attr = $dom->createAttribute('attr');
10+
$attr->textContent = "test";
11+
12+
try {
13+
$attr->replaceChild($dom->createProcessingInstruction('pi'), $attr->firstChild);
14+
} catch (DOMException $e) {
15+
echo $e->getMessage(), "\n";
16+
}
17+
18+
$root = $dom->appendChild($dom->createElement('root'));
19+
$root->setAttributeNode($attr);
20+
21+
echo $dom->saveXML();
22+
23+
?>
24+
--EXPECT--
25+
Hierarchy Request Error
26+
<?xml version="1.0"?>
27+
<root attr="test"/>

‎ext/spl/spl_heap.c

+37-18
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#define PTR_HEAP_BLOCK_SIZE 64
3131

3232
#define SPL_HEAP_CORRUPTED 0x00000001
33+
#define SPL_HEAP_WRITE_LOCKED 0x00000002
3334

3435
static zend_object_handlers spl_handler_SplHeap;
3536
static zend_object_handlers spl_handler_SplPriorityQueue;
@@ -276,12 +277,16 @@ static void spl_ptr_heap_insert(spl_ptr_heap *heap, void *elem, void *cmp_userda
276277
heap->max_size *= 2;
277278
}
278279

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

288+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
289+
285290
if (EG(exception)) {
286291
/* exception thrown during comparison */
287292
heap->flags |= SPL_HEAP_CORRUPTED;
@@ -309,6 +314,8 @@ static zend_result spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void
309314
return FAILURE;
310315
}
311316

317+
heap->flags |= SPL_HEAP_WRITE_LOCKED;
318+
312319
if (elem) {
313320
spl_heap_elem_copy(heap, elem, spl_heap_elem(heap, 0));
314321
} else {
@@ -332,6 +339,8 @@ static zend_result spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void
332339
}
333340
}
334341

342+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
343+
335344
if (EG(exception)) {
336345
/* exception thrown during comparison */
337346
heap->flags |= SPL_HEAP_CORRUPTED;
@@ -377,10 +386,14 @@ static void spl_ptr_heap_destroy(spl_ptr_heap *heap) { /* {{{ */
377386

378387
int i;
379388

389+
heap->flags |= SPL_HEAP_WRITE_LOCKED;
390+
380391
for (i = 0; i < heap->count; ++i) {
381392
heap->dtor(spl_heap_elem(heap, i));
382393
}
383394

395+
heap->flags &= ~SPL_HEAP_WRITE_LOCKED;
396+
384397
efree(heap->elements);
385398
efree(heap);
386399
}
@@ -589,6 +602,21 @@ PHP_METHOD(SplHeap, isEmpty)
589602
}
590603
/* }}} */
591604

605+
static zend_result spl_heap_consistency_validations(const spl_heap_object *intern, bool write)
606+
{
607+
if (intern->heap->flags & SPL_HEAP_CORRUPTED) {
608+
zend_throw_exception(spl_ce_RuntimeException, "Heap is corrupted, heap properties are no longer ensured.", 0);
609+
return FAILURE;
610+
}
611+
612+
if (write && (intern->heap->flags & SPL_HEAP_WRITE_LOCKED)) {
613+
zend_throw_exception(spl_ce_RuntimeException, "Heap cannot be changed when it is already being modified.", 0);
614+
return FAILURE;
615+
}
616+
617+
return SUCCESS;
618+
}
619+
592620
/* {{{ Push $value on the heap */
593621
PHP_METHOD(SplHeap, insert)
594622
{
@@ -601,8 +629,7 @@ PHP_METHOD(SplHeap, insert)
601629

602630
intern = Z_SPLHEAP_P(ZEND_THIS);
603631

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

@@ -624,8 +651,7 @@ PHP_METHOD(SplHeap, extract)
624651

625652
intern = Z_SPLHEAP_P(ZEND_THIS);
626653

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

@@ -650,8 +676,7 @@ PHP_METHOD(SplPriorityQueue, insert)
650676

651677
intern = Z_SPLHEAP_P(ZEND_THIS);
652678

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

@@ -691,8 +716,7 @@ PHP_METHOD(SplPriorityQueue, extract)
691716

692717
intern = Z_SPLHEAP_P(ZEND_THIS);
693718

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

@@ -718,8 +742,7 @@ PHP_METHOD(SplPriorityQueue, top)
718742

719743
intern = Z_SPLHEAP_P(ZEND_THIS);
720744

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

@@ -829,8 +852,7 @@ PHP_METHOD(SplHeap, top)
829852

830853
intern = Z_SPLHEAP_P(ZEND_THIS);
831854

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

@@ -894,8 +916,7 @@ static zval *spl_heap_it_get_current_data(zend_object_iterator *iter) /* {{{ */
894916
{
895917
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);
896918

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

@@ -912,8 +933,7 @@ static zval *spl_pqueue_it_get_current_data(zend_object_iterator *iter) /* {{{ *
912933
zend_user_iterator *user_it = (zend_user_iterator *) iter;
913934
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);
914935

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

@@ -941,8 +961,7 @@ static void spl_heap_it_move_forward(zend_object_iterator *iter) /* {{{ */
941961
{
942962
spl_heap_object *object = Z_SPLHEAP_P(&iter->data);
943963

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

‎ext/spl/tests/gh16337.phpt

+49
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)