[php-src] master: Fix GH-22554: ext/dom: UAF with XPath callback returning foreign-doc node

Author: David Carlier (devnexen) Date: 2026-10-23T16:55:05+01:00 Commit: https://github.com/php/php-src/commit/a44a529d266b0152d90553582875a77445b43233 Raw diff: https://github.com/php/php-src/commit/a44a529d266b0152d90553582875a77445b43233.diff Fix GH-22554: ext/dom: UAF with XPath callback returning foreign-doc node A PHP XPath callback that returns a node belonging to a document created inside the callback (e.g. $d->documentElement of a throwaway DOMDocument) parks that node in the DOMXPath node_list to keep it alive. When a sibling callback consumes a node navigated into that foreign document, the proxy object was created with the DOMXPath's own dom as parent, so it took a reference on the wrong document and none on the foreign one. On teardown the foreign document could be freed while the proxy still referenced it. Route the proxy factory through dom_xpath_intern_for_doc() so the created object shares the ref_obj of the node's actual document, mirroring the query-result path. Also backports the GH-22077 fix (UAF in custom XPath function) and the DOMXPath crash fix when wrapping a foreign node after a nodeset callback. (cherry picked from commit 33a49bb4d390ad901ba900683a399363ce4e32fe) (cherry picked from commit f36660d64507e0d5d9fce73739031faac8139ded) (cherry picked from commit fcdc4fbf858c91ddfaa5d4203ea279737a102a19) Co-authored-by: Joe Afflerbach <git@afflerbach.info> Co-authored-by: Ilia Alshanetsky <ilia@ilia.ws> Close GH-24570 Changed paths: A ext/dom/tests/gh22077.phpt A ext/dom/tests/gh22554.phpt A ext/dom/tests/xpath_php_function_foreign_doc.phpt M NEWS M ext/dom/xpath.c Diff: diff --git a/NEWS b/NEWS index 8bd02dc6d6ac..5bf9f52d661e 100644 --- a/NEWS +++ b/NEWS @@ -40,6 +40,11 @@ PHP NEWS selector lists). (fivefilters) . Fixed bug GH-23334 (Use-after-free in php_dom_free_templated_content()). (David Carlier, ndossche) + . Fixed bug GH-22077 (UAF in custom XPath function). (Joe Afflerbach) + . Fixed bug GH-22554 (use-after-free with XPath callback returning a node + from a foreign document). (David Carlier) + . Fixed DOMXPath crash wrapping a foreign node after a nodeset callback. + (Ilia Alshanetsky) - FPM: . Fixed bug GH-24229 (fcgi_close() passes shutdown() a literal 1, which is diff --git a/ext/dom/tests/gh22077.phpt b/ext/dom/tests/gh22077.phpt new file mode 100644 index 000000000000..fd4e42cc8aaf --- /dev/null +++ b/ext/dom/tests/gh22077.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-22077 (UAF in custom XPath function) +--FILE-- +<?php +$document = new DOMDocument; +$xpath = new DOMXPath($document); +$xpath->registerNamespace("my", "my.ns"); +$xpath->registerPHPFunctionNS('my.ns', 'include', function(): DOMElement { + $includedDocument = new DOMDocument; + $includedDocument->loadXML('<root><uaf/><node/><uaf/></root>'); + return $includedDocument->documentElement; +}); +$nodeset = $xpath->query('my:include()/uaf'); +$node = $nodeset->item(0); +var_dump($nodeset->length); +var_dump($node->ownerDocument->saveXML($node)); +?> +--EXPECT-- +int(2) +string(6) "<uaf/>" diff --git a/ext/dom/tests/gh22554.phpt b/ext/dom/tests/gh22554.phpt new file mode 100644 index 000000000000..c8db1d87390b --- /dev/null +++ b/ext/dom/tests/gh22554.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-22554 (Use-after-free when an XPath callback returns a node from a document created inside the callback) +--CREDITS-- +waseem-cve +--EXTENSIONS-- +dom +--FILE-- +<?php + +$doc = new DOMDocument; +$doc->loadXML('<root/>'); + +$xp = new DOMXPath($doc); +$xp->registerNamespace('my', 'my.ns'); + +$xp->registerPHPFunctionNS('my.ns', 'include', function () { + $d = new DOMDocument; + $d->loadXML('<r><uaf/></r>'); + + return $d->documentElement; +}); + +$xp->registerPHPFunctionNS('my.ns', 'process', function ($arg) { + echo "process arg: ", get_class($arg[0]), " ", $arg[0]->nodeName, "\n"; + return 'x'; +}); + +$result = $xp->query('my:process(my:include()/uaf)'); +var_dump($result->length); +unset($xp); + +echo "Done\n"; + +?> +--EXPECT-- +process arg: DOMElement uaf +int(0) +Done diff --git a/ext/dom/tests/xpath_php_function_foreign_doc.phpt b/ext/dom/tests/xpath_php_function_foreign_doc.phpt new file mode 100644 index 000000000000..4e4b2b2c2822 --- /dev/null +++ b/ext/dom/tests/xpath_php_function_foreign_doc.phpt @@ -0,0 +1,40 @@ +--TEST-- +DOMXPath: php:function nodeset args plus a foreign-document return must not treat arrays as DOM objects +--EXTENSIONS-- +dom +--FILE-- +<?php +$doc1 = new DOMDocument(); +$doc1->loadXML('<root><a>1</a></root>'); +$doc2 = new DOMDocument(); +$doc2->loadXML('<root><b>2</b></root>'); + +$xp = new DOMXPath($doc1); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function uses_nodeset($nodes) { + return true; +} + +function foreign() { + global $doc2; + return $doc2->documentElement; +} + +$xp->query('//a[php:function("uses_nodeset", //a)]'); +$res = $xp->query('php:function("foreign")'); +echo "count: "; +var_dump($res->length); +$n = $res->item(0); +echo "name: "; +var_dump($n->nodeName); +echo "owner is doc2: "; +var_dump($n->ownerDocument === $doc2); +echo "done\n"; +?> +--EXPECT-- +count: int(1) +name: string(4) "root" +owner is doc2: bool(true) +done diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index c3d15fe44ddb..a9832f4c3e22 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -34,6 +34,43 @@ #ifdef LIBXML_XPATH_ENABLED +static dom_object *dom_xpath_intern_from_entry(zval *entry, xmlDocPtr doc) +{ + if (Z_TYPE_P(entry) == IS_OBJECT) { + dom_object *obj = Z_DOMOBJ_P(entry); + if (obj->document && obj->document->ptr == doc) { + return obj; + } + } else if (Z_TYPE_P(entry) == IS_ARRAY) { + zval *inner; + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(entry), inner) { + dom_object *obj = dom_xpath_intern_from_entry(inner, doc); + if (obj) { + return obj; + } + } ZEND_HASH_FOREACH_END(); + } + return NULL; +} + +static dom_object *dom_xpath_intern_for_doc(dom_xpath_object *xpath_obj, xmlDocPtr doc) +{ + if (xpath_obj->dom.document && xpath_obj->dom.document->ptr == doc) { + return &xpath_obj->dom; + } + HashTable *node_list = xpath_obj->xpath_callbacks.node_list; + if (node_list) { + zval *entry; + ZEND_HASH_PACKED_FOREACH_VAL(node_list, entry) { + dom_object *obj = dom_xpath_intern_from_entry(entry, doc); + if (obj) { + return obj; + } + } ZEND_HASH_FOREACH_END(); + } + return &xpath_obj->dom; +} + void dom_xpath_objects_free_storage(zend_object *object) { dom_xpath_object *intern = php_xpath_obj_from_obj(object); @@ -60,7 +97,8 @@ static void dom_xpath_proxy_factory(xmlNodePtr node, zval *child, dom_object *in ZEND_ASSERT(node->type != XML_NAMESPACE_DECL); - php_dom_create_object(node, child, intern); + dom_xpath_object *xobj = php_xpath_obj_from_obj(&intern->std); + php_dom_create_object(node, child, dom_xpath_intern_for_doc(xobj, node->doc)); } static dom_xpath_object *dom_xpath_ext_fetch_intern(xmlXPathParserContextPtr ctxt) @@ -378,7 +416,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern) node = php_dom_create_fake_namespace_decl(nsparent, original, &child, parent_intern); } else { - php_dom_create_object(node, &child, &intern->dom); + dom_object *parent = dom_xpath_intern_for_doc(intern, node->doc); + php_dom_create_object(node, &child, parent); } add_next_index_zval(&retval, &child); if (ZEND_HAS_OOM()) {