Merge branch 'PHP-8.4'

* PHP-8.4: Fix phpGH-16595: Another UAF in DOM -> cloneNode Fix phpGH-16593: Assertion failure in DOM->replaceChild
TimWolla · Oct 28, 2024 · 99cdd67 · 99cdd67
2 parents 91270aa + 6e82ae9
commit 99cdd67
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 7 deletions.
diff --git a/ext/dom/node.c b/ext/dom/node.c
@@ -895,21 +895,23 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
 		RETURN_FALSE;
 	}
 
-	if (child->doc == NULL && parentp->doc != NULL) {
-		dom_set_document_ref_pointers(child, intern->document);
-	}
-
-	php_libxml_invalidate_node_list_cache(intern->document);
-
+	xmlNodePtr refp = NULL;
 	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->doc == NULL && parentp->doc != NULL) {
+		dom_set_document_ref_pointers(child, intern->document);
+	}
+
+	php_libxml_invalidate_node_list_cache(intern->document);
 
+	if (ref != NULL) {
 		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
@@ -0,0 +1,22 @@
+--TEST--
+GH-16593 (Assertion failure in DOM->replaceChild)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument;
+$root = $doc->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
+<?xml version="1.0"?>
+<root><child/></root>
diff --git a/ext/dom/tests/gh16595.phpt b/ext/dom/tests/gh16595.phpt
@@ -0,0 +1,26 @@
+--TEST--
+GH-16595 (Another UAF in DOM -> cloneNode)
+--EXTENSIONS--
+dom
+--CREDITS--
+chibinz
+--FILE--
+<?php
+$v0 = new DOMElement ( "jg" );
+$v1 = new DOMDocument ( "Zb" );
+$v2 = new DOMElement ( "IU" );
+$v7 = new DOMElement ( "L" , null , "df" );
+$v9 = new DOMDocument (  );
+
+try { $v1 -> insertBefore ( $v0 , $v9 ); } catch (\Throwable) { }
+$v0 -> replaceChildren ( $v7 );
+$v7 -> before ( $v2 );
+$v1 -> insertBefore ( $v0 );
+$v2 -> cloneNode (  );
+echo $v1->saveXML();
+echo $v9->saveXML();
+?>
+--EXPECT--
+<?xml version="Zb"?>
+<jg xmlns:default="df"><IU/><default:L xmlns="df"/></jg>
+<?xml version="1.0"?>