Commit d4235d7cf91 for php
commit d4235d7cf916a165485e04e85cc56e95d42727e7
Author: David Carlier <devnexen@gmail.com>
Date: Tue Aug 18 20:28:09 2026 +0100
Fix GH-23352: DOMDocument::adoptNode() stale document references
php_dom_transfer_document_ref() only retargeted the leftmost descendant
chain, and never an attribute's value nodes, leaving retained nodes
pointing at the freed source document. Replaced with an iterative walk
over the whole subtree.
Close GH-23358
diff --git a/NEWS b/NEWS
index e8b86a95e07..fd9ab08cd72 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 8.4.28
+- DOM:
+ . Fixed bug GH-23352 (UAF reading an attribute value node retained across
+ DOMDocument::adoptNode()). (David Carlier)
+
- Opcache:
. Fixed bug GH-20890 (Segfault in zval_undefined_cv with non-simple property
hook with minimal tracing JIT). (ndossche)
diff --git a/ext/dom/document.c b/ext/dom/document.c
index 9c269e4cb14..377aee8029c 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -1086,21 +1086,26 @@ static zend_always_inline void php_dom_transfer_document_ref_single_node(xmlNode
}
}
-static void php_dom_transfer_document_ref(xmlNodePtr node, php_libxml_ref_obj *new_document)
+static zend_always_inline void php_dom_transfer_document_ref_single_aux(xmlNodePtr node, php_libxml_ref_obj *new_document)
{
- if (node->children) {
- php_dom_transfer_document_ref(node->children, new_document);
- }
-
- while (node) {
- if (node->type == XML_ELEMENT_NODE) {
- for (xmlAttrPtr attr = node->properties; attr != NULL; attr = attr->next) {
- php_dom_transfer_document_ref_single_node((xmlNodePtr) attr, new_document);
+ php_dom_transfer_document_ref_single_node(node, new_document);
+ if (node->type == XML_ELEMENT_NODE) {
+ for (xmlAttrPtr attr = node->properties; attr; attr = attr->next) {
+ php_dom_transfer_document_ref_single_node((xmlNodePtr) attr, new_document);
+ for (xmlNodePtr child = attr->children; child; child = child->next) {
+ php_dom_transfer_document_ref_single_node((xmlNodePtr) child, new_document);
}
}
+ }
+}
- php_dom_transfer_document_ref_single_node(node, new_document);
- node = node->next;
+static void php_dom_transfer_document_ref(xmlNodePtr node, php_libxml_ref_obj *new_document)
+{
+ php_dom_transfer_document_ref_single_aux(node, new_document);
+ xmlNodePtr tmp = node->children;
+ while (tmp) {
+ php_dom_transfer_document_ref_single_aux(tmp, new_document);
+ tmp = php_dom_next_in_tree_order(tmp, node);
}
}
diff --git a/ext/dom/tests/DOMDocument_adoptNode_sibling_subtree.phpt b/ext/dom/tests/DOMDocument_adoptNode_sibling_subtree.phpt
new file mode 100644
index 00000000000..d794324fc89
--- /dev/null
+++ b/ext/dom/tests/DOMDocument_adoptNode_sibling_subtree.phpt
@@ -0,0 +1,22 @@
+--TEST--
+DOMDocument::adoptNode() with a node retained under a later sibling
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$source = new DOMDocument();
+$root = $source->appendChild($source->createElement('root'));
+$root->appendChild($source->createElement('first'));
+$second = $root->appendChild($source->createElement('second'));
+$victim = $second->appendChild($source->createElement('grandchild'));
+
+$destination = new DOMDocument();
+$destination->appendChild($destination->adoptNode($root));
+unset($destination, $source, $root, $second);
+
+echo $victim->nodeName, PHP_EOL;
+
+?>
+--EXPECT--
+grandchild
diff --git a/ext/dom/tests/gh23352.phpt b/ext/dom/tests/gh23352.phpt
new file mode 100644
index 00000000000..67d6d4be2ff
--- /dev/null
+++ b/ext/dom/tests/gh23352.phpt
@@ -0,0 +1,21 @@
+--TEST--
+GH-23352 (UAF reading an attribute value node retained across DOMDocument::adoptNode())
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$source = new DOMDocument();
+$element = $source->appendChild($source->createElement('element'));
+$element->setAttribute('attribute', 'victim');
+$victim = $element->getAttributeNode('attribute')->firstChild;
+
+$destination = new DOMDocument();
+$destination->appendChild($destination->adoptNode($element));
+unset($destination, $source, $element);
+
+echo $victim->data, PHP_EOL;
+
+?>
+--EXPECT--
+victim
diff --git a/ext/dom/tests/modern/spec/gh23352.phpt b/ext/dom/tests/modern/spec/gh23352.phpt
new file mode 100644
index 00000000000..f6d72dc9f18
--- /dev/null
+++ b/ext/dom/tests/modern/spec/gh23352.phpt
@@ -0,0 +1,21 @@
+--TEST--
+GH-23352 (UAF reading an attribute value node retained across Dom\Document::adoptNode())
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$source = Dom\XMLDocument::createFromString('<root/>');
+$element = $source->documentElement;
+$element->setAttribute('attribute', 'victim');
+$victim = $element->getAttributeNode('attribute')->firstChild;
+
+$destination = Dom\XMLDocument::createEmpty();
+$destination->appendChild($destination->adoptNode($element));
+unset($destination, $source, $element);
+
+echo $victim->data, PHP_EOL;
+
+?>
+--EXPECT--
+victim