diff --git a/NEWS b/NEWS index 4926783cc43..71e619788d0 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ PHP NEWS - DOM: . Fixed bug GH-16594 (Assertion failure in DOM -> before). (nielsdos) + . Fixed bug GH-16593 (Assertion failure in DOM->replaceChild). (nielsdos) + . Fixed bug GH-16595 (Another UAF in DOM -> cloneNode). (nielsdos) - GD: . Fixed bug GH-16559 (UBSan abort in ext/gd/libgd/gd_interpolation.c:1007). diff --git a/ext/dom/node.c b/ext/dom/node.c index c0fd56a472a..55d3e244547 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -895,6 +895,16 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj RETURN_FALSE; } + xmlNodePtr refp = NULL; + if (ref != NULL) { + dom_object *refpobj; + DOM_GET_OBJ(refp, ref, xmlNodePtr, refpobj); + if (refp->parent != parentp) { + php_dom_throw_error(NOT_FOUND_ERR, stricterror); + RETURN_FALSE; + } + } + if (child->doc == NULL && parentp->doc != NULL) { dom_set_document_ref_pointers(child, intern->document); } @@ -902,14 +912,6 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj php_libxml_invalidate_node_list_cache(intern->document); if (ref != NULL) { - xmlNodePtr refp; - dom_object *refpobj; - DOM_GET_OBJ(refp, ref, xmlNodePtr, refpobj); - if (refp->parent != parentp) { - php_dom_throw_error(NOT_FOUND_ERR, stricterror); - RETURN_FALSE; - } - if (child->parent != NULL) { xmlUnlinkNode(child); } @@ -1196,6 +1198,13 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern) RETURN_FALSE; } + /* This is already disallowed by libxml, but we should check it here to avoid + * breaking assumptions and assertions. */ + if ((oldchild->type == XML_ATTRIBUTE_NODE) != (newchild->type == XML_ATTRIBUTE_NODE)) { + php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + RETURN_FALSE; + } + if (oldchild->parent != nodep) { php_dom_throw_error(NOT_FOUND_ERR, stricterror); RETURN_FALSE; diff --git a/ext/dom/tests/gh16593.phpt b/ext/dom/tests/gh16593.phpt new file mode 100644 index 00000000000..416d794a4f0 --- /dev/null +++ b/ext/dom/tests/gh16593.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-16593 (Assertion failure in DOM->replaceChild) +--EXTENSIONS-- +dom +--FILE-- +appendChild($doc->createElement('root')); +$child = $root->appendChild($doc->createElement('child')); +try { + $root->replaceChild($doc->createAttribute('foo'), $child); +} catch (DOMException $e) { + echo $e->getMessage(), "\n"; +} +echo $doc->saveXML(); + +?> +--EXPECT-- +Hierarchy Request Error + + diff --git a/ext/dom/tests/gh16595.phpt b/ext/dom/tests/gh16595.phpt new file mode 100644 index 00000000000..98927b2ebe9 --- /dev/null +++ b/ext/dom/tests/gh16595.phpt @@ -0,0 +1,26 @@ +--TEST-- +GH-16595 (Another UAF in DOM -> cloneNode) +--EXTENSIONS-- +dom +--CREDITS-- +chibinz +--FILE-- + insertBefore ( $v0 , $v9 ); } catch (\Throwable) { } +$v0 -> replaceChildren ( $v7 ); +$v7 -> before ( $v2 ); +$v1 -> insertBefore ( $v0 ); +$v2 -> cloneNode ( ); +echo $v1->saveXML(); +echo $v9->saveXML(); +?> +--EXPECT-- + + +