Skip to content

Commit 21e2c56

Browse files
committed
dom: clear the xpath callback registrations before freeing them
php_dom_xpath_callbacks_dtor() freed php_ns and each namespaces entry while leaving registry->php_ns and registry->namespaces pointing at them, and it then destroyed node_list, which runs node destructors. A destructor calling gc_collect_cycles() therefore reached php_dom_xpath_callback_ns_get_gc() through the still-set fields and iterated freed memory. Reachable from userland by calling DOMXPath::__construct() a second time on an object that has php:function registrations and a populated node list. Closes GH-23621
1 parent 7ef3bfe commit 21e2c56

4 files changed

Lines changed: 114 additions & 5 deletions

File tree

NEWS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.4.27
44

5+
- DOM:
6+
. Fixed use-after-free when re-constructing a DOMXPath whose php:function
7+
registrations are freed while still reachable from the cycle collector.
8+
(Ilia Alshanetsky)
9+
510

611
24 Sep 2026, PHP 8.4.26
712

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
Re-constructing a DOMXPath does not expose freed php:function registrations to the cycle collector
3+
--EXTENSIONS--
4+
dom
5+
--ENV--
6+
USE_ZEND_ALLOC=0
7+
--FILE--
8+
<?php
9+
class GcElement extends DOMElement
10+
{
11+
public function __destruct()
12+
{
13+
gc_collect_cycles();
14+
}
15+
}
16+
17+
class Holder
18+
{
19+
public $self;
20+
21+
public function cb($node)
22+
{
23+
return true;
24+
}
25+
}
26+
27+
$doc = new DOMDocument();
28+
$doc->loadXML('<r><a/><b/><c/></r>');
29+
$doc->registerNodeClass(DOMElement::class, GcElement::class);
30+
31+
$xp = new DOMXPath($doc);
32+
$xp->registerNamespace('php', 'http://php.net/xpath');
33+
34+
$holder = new Holder();
35+
$holder->self = $holder;
36+
$xp->registerPhpFunctions(['cb' => [$holder, 'cb']]);
37+
38+
$xp->query('/r/*[php:function("cb", .)]');
39+
unset($holder);
40+
41+
/* Make the object a collector root candidate, then re-construct it: the
42+
registration teardown must not stay reachable while it is being freed. */
43+
$tmp = $xp;
44+
unset($tmp);
45+
$xp->__construct($doc);
46+
47+
var_dump($xp->query('/r/a')->length);
48+
?>
49+
--EXPECT--
50+
int(1)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
Re-constructing a DOMXPath does not expose freed namespaced php:function registrations to the cycle collector
3+
--EXTENSIONS--
4+
dom
5+
--ENV--
6+
USE_ZEND_ALLOC=0
7+
--FILE--
8+
<?php
9+
class GcElement extends DOMElement
10+
{
11+
public function __destruct()
12+
{
13+
gc_collect_cycles();
14+
}
15+
}
16+
17+
class Holder
18+
{
19+
public $self;
20+
21+
public function cb($node)
22+
{
23+
return true;
24+
}
25+
}
26+
27+
$doc = new DOMDocument();
28+
$doc->loadXML('<r><a/><b/><c/></r>');
29+
$doc->registerNodeClass(DOMElement::class, GcElement::class);
30+
31+
$xp = new DOMXPath($doc);
32+
$xp->registerNamespace('my', 'urn:my');
33+
34+
$holder = new Holder();
35+
$holder->self = $holder;
36+
$xp->registerPhpFunctionNS('urn:my', 'cb', [$holder, 'cb']);
37+
38+
$xp->query('/r/*[my:cb(.)]');
39+
unset($holder);
40+
41+
/* Make the object a collector root candidate, then re-construct it: the
42+
registration teardown must not stay reachable while it is being freed. */
43+
$tmp = $xp;
44+
unset($tmp);
45+
$xp->__construct($doc);
46+
47+
var_dump($xp->query('/r/a')->length);
48+
?>
49+
--EXPECT--
50+
int(1)

ext/dom/xpath_callbacks.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,22 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_argument_stack(xmlXPathParserC
7676
PHP_DOM_EXPORT void php_dom_xpath_callbacks_dtor(php_dom_xpath_callbacks *registry)
7777
{
7878
if (registry->php_ns) {
79-
php_dom_xpath_callback_ns_dtor(registry->php_ns);
80-
efree(registry->php_ns);
79+
php_dom_xpath_callback_ns *php_ns = registry->php_ns;
80+
registry->php_ns = NULL;
81+
php_dom_xpath_callback_ns_dtor(php_ns);
82+
efree(php_ns);
8183
}
8284
if (registry->namespaces) {
85+
HashTable *namespaces = registry->namespaces;
86+
registry->namespaces = NULL;
8387
php_dom_xpath_callback_ns *ns;
84-
ZEND_HASH_MAP_FOREACH_PTR(registry->namespaces, ns) {
88+
ZEND_HASH_MAP_FOREACH_PTR(namespaces, ns) {
8589
php_dom_xpath_callback_ns_dtor(ns);
8690
efree(ns);
8791
} ZEND_HASH_FOREACH_END();
8892

89-
zend_hash_destroy(registry->namespaces);
90-
FREE_HASHTABLE(registry->namespaces);
93+
zend_hash_destroy(namespaces);
94+
FREE_HASHTABLE(namespaces);
9195
}
9296
php_dom_xpath_callbacks_clean_node_list(registry);
9397
}

0 commit comments

Comments
 (0)