diff --git a/NEWS b/NEWS index a67a09e081d..e926ab18dd6 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,7 @@ PHP NEWS - DOM: . Fixed bug GH-16316 (DOMXPath breaks when not initialized properly). (nielsdos) + . Add missing hierarchy checks to replaceChild. (nielsdos) - GD: . Fixed bug GH-16334 (imageaffine overflow on matrix elements). @@ -34,6 +35,9 @@ PHP NEWS . Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params). (nielsdos) +- SPL: + . Fixed bug GH-16337 (Use-after-free in SplHeap). (nielsdos) + - XMLReader: . Fixed bug GH-16292 (Segmentation fault in ext/xmlreader/php_xmlreader.c). (nielsdos) diff --git a/ext/dom/node.c b/ext/dom/node.c index 3e0a3b90510..f35eb074e35 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -810,7 +810,7 @@ static xmlNodePtr dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlN } /* }}} */ -static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNodePtr child, bool stricterror) +static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNodePtr child, bool stricterror, bool warn_empty_fragment) { if (dom_node_is_read_only(parentp) == SUCCESS || (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 return false; } - if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) { + if (warn_empty_fragment && child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) { /* TODO Drop Warning? */ php_error_docref(NULL, E_WARNING, "Document Fragment is empty"); return false; @@ -855,7 +855,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj xmlNodePtr new_child = NULL; bool stricterror = dom_get_strict_error(intern->document); - if (!dom_node_check_legacy_insertion_validity(parentp, child, stricterror)) { + if (!dom_node_check_legacy_insertion_validity(parentp, child, stricterror, true)) { RETURN_FALSE; } @@ -1152,18 +1152,7 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern) RETURN_FALSE; } - if (!nodep->children) { - RETURN_FALSE; - } - - if (dom_node_is_read_only(nodep) == SUCCESS || - (newchild->parent != NULL && dom_node_is_read_only(newchild->parent) == SUCCESS)) { - php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror); - RETURN_FALSE; - } - - if (dom_hierarchy(nodep, newchild) == FAILURE) { - php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + if (!dom_node_check_legacy_insertion_validity(nodep, newchild, stricterror, false)) { RETURN_FALSE; } @@ -1277,7 +1266,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern, bool stricterror = dom_get_strict_error(intern->document); - if (!dom_node_check_legacy_insertion_validity(nodep, child, stricterror)) { + if (!dom_node_check_legacy_insertion_validity(nodep, child, stricterror, true)) { RETURN_FALSE; } diff --git a/ext/dom/tests/replaceChild_attribute_validation.phpt b/ext/dom/tests/replaceChild_attribute_validation.phpt new file mode 100644 index 00000000000..32d5990f75e --- /dev/null +++ b/ext/dom/tests/replaceChild_attribute_validation.phpt @@ -0,0 +1,27 @@ +--TEST-- +replaceChild with attribute children +--EXTENSIONS-- +dom +--FILE-- +createAttribute('attr'); +$attr->textContent = "test"; + +try { + $attr->replaceChild($dom->createProcessingInstruction('pi'), $attr->firstChild); +} catch (DOMException $e) { + echo $e->getMessage(), "\n"; +} + +$root = $dom->appendChild($dom->createElement('root')); +$root->setAttributeNode($attr); + +echo $dom->saveXML(); + +?> +--EXPECT-- +Hierarchy Request Error + + diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c index 1fdc2c5bdf9..d4450da4200 100644 --- a/ext/spl/spl_heap.c +++ b/ext/spl/spl_heap.c @@ -30,6 +30,7 @@ #define PTR_HEAP_BLOCK_SIZE 64 #define SPL_HEAP_CORRUPTED 0x00000001 +#define SPL_HEAP_WRITE_LOCKED 0x00000002 static zend_object_handlers spl_handler_SplHeap; 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 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; @@ -309,6 +314,8 @@ static zend_result spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void return FAILURE; } + heap->flags |= SPL_HEAP_WRITE_LOCKED; + if (elem) { spl_heap_elem_copy(heap, elem, spl_heap_elem(heap, 0)); } else { @@ -332,6 +339,8 @@ static zend_result spl_ptr_heap_delete_top(spl_ptr_heap *heap, void *elem, void } } + heap->flags &= ~SPL_HEAP_WRITE_LOCKED; + if (EG(exception)) { /* exception thrown during comparison */ heap->flags |= SPL_HEAP_CORRUPTED; @@ -377,10 +386,14 @@ 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); } @@ -589,6 +602,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) { @@ -601,8 +629,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(); } @@ -624,8 +651,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(); } @@ -650,8 +676,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(); } @@ -691,8 +716,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(); } @@ -718,8 +742,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(); } @@ -829,8 +852,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(); } @@ -894,8 +916,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; } @@ -912,8 +933,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; } @@ -941,8 +961,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; } diff --git a/ext/spl/tests/gh16337.phpt b/ext/spl/tests/gh16337.phpt new file mode 100644 index 00000000000..94cf9d90cb1 --- /dev/null +++ b/ext/spl/tests/gh16337.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-16337 (Use-after-free in SplHeap) +--FILE-- +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