diff --git a/NEWS b/NEWS index 1aaca5dc75b..1ccb911b779 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,8 @@ PHP NEWS . Fixed bug GH-16533 (Segfault when adding attribute to parent that is not an element). (nielsdos) . Fixed bug GH-16535 (UAF when using document as a child). (nielsdos) + . Fixed bug GH-16593 (Assertion failure in DOM->replaceChild). (nielsdos) + . Fixed bug GH-16595 (Another UAF in DOM -> cloneNode). (nielsdos) - EXIF: . Fixed bug GH-16409 (Segfault in exif_thumbnail when not dealing with a diff --git a/ext/dom/node.c b/ext/dom/node.c index beccd257e61..cc693cd8a36 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -990,7 +990,7 @@ Since: PHP_METHOD(DOMNode, insertBefore) { zval *id, *node, *ref = NULL; - xmlNodePtr child, new_child, parentp, refp; + xmlNodePtr child, new_child, parentp, refp = NULL; dom_object *intern, *childobj, *refpobj; int ret, stricterror; @@ -1015,6 +1015,14 @@ PHP_METHOD(DOMNode, insertBefore) RETURN_FALSE; } + if (ref != NULL) { + 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); } @@ -1022,12 +1030,6 @@ PHP_METHOD(DOMNode, insertBefore) php_libxml_invalidate_node_list_cache(intern->document); if (ref != NULL) { - 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); } @@ -1173,6 +1175,13 @@ PHP_METHOD(DOMNode, replaceChild) 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 + +