From d89dd28d3b6595757596fd174e31e61ef2bd89b7 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 25 Oct 2024 18:59:45 +0200
Subject: [PATCH 1/2] Fix GH-16593: Assertion failure in DOM->replaceChild

This is already forbidden by libxml, but this condition isn't properly
checked; so the return value and lack of error makes it seem like it
worked while it actually didn't. Furthermore, this can break assumptions
and assertions later on.

Closes GH-16596.
---
 NEWS                       |  1 +
 ext/dom/node.c             |  7 +++++++
 ext/dom/tests/gh16593.phpt | 22 ++++++++++++++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 ext/dom/tests/gh16593.phpt

diff --git a/NEWS b/NEWS
index 08db61d58eb..11cf6cd0214 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,7 @@ 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)
 
 - 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 a8e3b035d14..188806564c6 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -1093,6 +1093,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--
+<?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>

From 9d8983c061084d9d33d34ea5392e6155f31fb03f 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 2/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.

Closes GH-16598.
---
 NEWS           |  1 +
 ext/dom/node.c | 15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 11cf6cd0214..857248b413e 100644
--- a/NEWS
+++ b/NEWS
@@ -40,6 +40,7 @@ PHP                                                                        NEWS
     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 188806564c6..fc668415dae 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -893,7 +893,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;
 
@@ -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);
 		}