[php-src] PHP-8.6: Fix GH-23352: DOMDocument::adoptNode() stale document references

Author: David Carlier (devnexen) Date: 2026-10-06T22:44:00+01:00 Commit: https://github.com/php/php-src/commit/d4235d7cf916a165485e04e85cc56e95d42727e7 Raw diff: https://github.com/php/php-src/commit/d4235d7cf916a165485e04e85cc56e95d42727e7.diff 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 Changed paths: A ext/dom/tests/DOMDocument_adoptNode_sibling_subtree.phpt A ext/dom/tests/gh23352.phpt A ext/dom/tests/modern/spec/gh23352.phpt M NEWS M ext/dom/document.c Diff: diff --git a/NEWS b/NEWS index e8b86a95e07e..fd9ab08cd728 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 9c269e4cb14d..377aee8029cb 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 000000000000..d794324fc899 --- /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 000000000000..67d6d4be2ffb --- /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 000000000000..f6d72dc9f182 --- /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