From ac5cdb9c1d934216aa215a682286032161de6ad8 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 25 Oct 2024 19:27:02 +0200 Subject: [PATCH 1/2] Fix GH-16595: Another UAF in DOM -> cloneNode We need to perform all sanity checks before doing any modification. I don't have a reliable and easy test for this on 8.2, but I have one for 8.4. --- ext/dom/node.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ext/dom/node.c b/ext/dom/node.c index a8e3b035d14c1..9d1a406a71688 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -918,18 +918,21 @@ PHP_METHOD(DOMNode, insertBefore) RETURN_FALSE; } - if (child->doc == NULL && parentp->doc != NULL) { - childobj->document = intern->document; - php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL); - } - + /* Fetch and perform sanity checks before modifying reference pointers. */ 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) { + childobj->document = intern->document; + php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL); + } + + if (ref != NULL) { if (child->parent != NULL) { xmlUnlinkNode(child); } From 8c2175b2eb761c7750896650040c533236e5452a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 25 Oct 2024 19:47:18 +0200 Subject: [PATCH 2/2] Silence false compiler warning --- ext/dom/node.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/dom/node.c b/ext/dom/node.c index 9d1a406a71688..675d61db35895 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -893,7 +893,7 @@ static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNode 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;