diff --git a/NEWS b/NEWS index ade78480b215..3346d38ea898 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,11 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.27 +- DOM: + . Fixed use-after-free when re-constructing a DOMXPath whose php:function + registrations are freed while still reachable from the cycle collector. + (Ilia Alshanetsky) + 24 Sep 2026, PHP 8.4.26 diff --git a/ext/dom/tests/DOMXPath_reconstruct_callbacks_gc.phpt b/ext/dom/tests/DOMXPath_reconstruct_callbacks_gc.phpt new file mode 100644 index 000000000000..224c6c16f67a --- /dev/null +++ b/ext/dom/tests/DOMXPath_reconstruct_callbacks_gc.phpt @@ -0,0 +1,50 @@ +--TEST-- +Re-constructing a DOMXPath does not expose freed php:function registrations to the cycle collector +--EXTENSIONS-- +dom +--ENV-- +USE_ZEND_ALLOC=0 +--FILE-- +loadXML(''); +$doc->registerNodeClass(DOMElement::class, GcElement::class); + +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); + +$holder = new Holder(); +$holder->self = $holder; +$xp->registerPhpFunctions(['cb' => [$holder, 'cb']]); + +$xp->query('/r/*[php:function("cb", .)]'); +unset($holder); + +/* Make the object a collector root candidate, then re-construct it: the + registration teardown must not stay reachable while it is being freed. */ +$tmp = $xp; +unset($tmp); +$xp->__construct($doc); + +var_dump($xp->query('/r/a')->length); +?> +--EXPECT-- +int(1) diff --git a/ext/dom/tests/DOMXPath_reconstruct_callbacks_ns_gc.phpt b/ext/dom/tests/DOMXPath_reconstruct_callbacks_ns_gc.phpt new file mode 100644 index 000000000000..13900d297ced --- /dev/null +++ b/ext/dom/tests/DOMXPath_reconstruct_callbacks_ns_gc.phpt @@ -0,0 +1,50 @@ +--TEST-- +Re-constructing a DOMXPath does not expose freed namespaced php:function registrations to the cycle collector +--EXTENSIONS-- +dom +--ENV-- +USE_ZEND_ALLOC=0 +--FILE-- +loadXML(''); +$doc->registerNodeClass(DOMElement::class, GcElement::class); + +$xp = new DOMXPath($doc); +$xp->registerNamespace('my', 'urn:my'); + +$holder = new Holder(); +$holder->self = $holder; +$xp->registerPhpFunctionNS('urn:my', 'cb', [$holder, 'cb']); + +$xp->query('/r/*[my:cb(.)]'); +unset($holder); + +/* Make the object a collector root candidate, then re-construct it: the + registration teardown must not stay reachable while it is being freed. */ +$tmp = $xp; +unset($tmp); +$xp->__construct($doc); + +var_dump($xp->query('/r/a')->length); +?> +--EXPECT-- +int(1) diff --git a/ext/dom/xpath_callbacks.c b/ext/dom/xpath_callbacks.c index 53e8f3443149..5dd3c5caded5 100644 --- a/ext/dom/xpath_callbacks.c +++ b/ext/dom/xpath_callbacks.c @@ -76,18 +76,22 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_argument_stack(xmlXPathParserC PHP_DOM_EXPORT void php_dom_xpath_callbacks_dtor(php_dom_xpath_callbacks *registry) { if (registry->php_ns) { - php_dom_xpath_callback_ns_dtor(registry->php_ns); - efree(registry->php_ns); + php_dom_xpath_callback_ns *php_ns = registry->php_ns; + registry->php_ns = NULL; + php_dom_xpath_callback_ns_dtor(php_ns); + efree(php_ns); } if (registry->namespaces) { + HashTable *namespaces = registry->namespaces; + registry->namespaces = NULL; php_dom_xpath_callback_ns *ns; - ZEND_HASH_MAP_FOREACH_PTR(registry->namespaces, ns) { + ZEND_HASH_MAP_FOREACH_PTR(namespaces, ns) { php_dom_xpath_callback_ns_dtor(ns); efree(ns); } ZEND_HASH_FOREACH_END(); - zend_hash_destroy(registry->namespaces); - FREE_HASHTABLE(registry->namespaces); + zend_hash_destroy(namespaces); + FREE_HASHTABLE(namespaces); } php_dom_xpath_callbacks_clean_node_list(registry); }