Skip to content

Commit a74f826

Browse files
committed
Fix GH-23729: DOMXPath::__construct() use-after-free during an evaluation
Reconstructing the object from a php:function callback freed the context libxml2 was still evaluating, and php_xpath_eval() wrote back into it once the evaluation returned. The evaluation depth is now tracked on the object and __construct() throws while it is non-zero. Close GH-23735
1 parent 132403d commit a74f826

4 files changed

Lines changed: 85 additions & 1 deletion

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ PHP NEWS
1313
. Fixed Dom\HTMLDocument giving attributes the namespace of their element
1414
when a fragment is parsed with an xlink, xml or xmlns context element.
1515
(Ilia Alshanetsky)
16+
. Fixed bug GH-23729 (DOMXPath::__construct() use-after-free during an
17+
evaluation). (David Carlier)
1618

1719
- Intl:
1820
. Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state

‎ext/dom/php_dom.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ extern zend_module_entry dom_module_entry;
6666

6767
typedef struct dom_xpath_object {
6868
php_dom_xpath_callbacks xpath_callbacks;
69+
uint32_t evaluation_depth;
6970
bool register_node_ns;
7071
dom_object dom;
7172
} dom_xpath_object;

‎ext/dom/tests/gh23729.phpt‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
--TEST--
2+
GH-23729 (Use-after-free when DOMXPath is reconstructed during an evaluation)
3+
--CREDITS--
4+
djarfluka
5+
--EXTENSIONS--
6+
dom
7+
--FILE--
8+
<?php
9+
10+
function reconstruct() {
11+
try {
12+
$GLOBALS['xpath']->__construct($GLOBALS['other']);
13+
} catch (Error $e) {
14+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
15+
}
16+
return 'r';
17+
}
18+
19+
function nested() {
20+
echo 'nested: ', $GLOBALS['xpath']->evaluate('string(/root/b)'), PHP_EOL;
21+
return 'n';
22+
}
23+
24+
function test(string $class, object $doc, object $other) {
25+
$xpath = new $class($doc);
26+
$xpath->registerNamespace('php', 'http://php.net/xpath');
27+
$xpath->registerPhpFunctions();
28+
29+
$GLOBALS['xpath'] = $xpath;
30+
$GLOBALS['other'] = $other;
31+
32+
var_dump($xpath->evaluate('string(php:function("reconstruct"))'));
33+
/* The evaluation the callback tried to destroy must still be usable. */
34+
var_dump($xpath->evaluate('string(/root/a)'));
35+
/* A nested evaluation must not lift the guard of the outer one. */
36+
var_dump($xpath->evaluate('concat(php:function("nested"), php:function("reconstruct"))'));
37+
var_dump($xpath->query('//b[php:function("reconstruct")]')->length);
38+
39+
/* Reconstructing outside of an evaluation is still allowed. */
40+
$xpath->__construct($other);
41+
var_dump($xpath->document->documentElement->nodeName);
42+
}
43+
44+
$doc = new DOMDocument();
45+
$doc->loadXML('<root><a>1</a><b>2</b></root>');
46+
$other = new DOMDocument();
47+
$other->loadXML('<other/>');
48+
test(DOMXPath::class, $doc, $other);
49+
50+
$doc = Dom\XMLDocument::createFromString('<root><a>1</a><b>2</b></root>');
51+
$other = Dom\XMLDocument::createFromString('<other/>');
52+
test(Dom\XPath::class, $doc, $other);
53+
54+
?>
55+
--EXPECT--
56+
Error: Cannot call DOMXPath::__construct() while an XPath evaluation is in progress
57+
string(1) "r"
58+
string(1) "1"
59+
nested: 2
60+
Error: Cannot call DOMXPath::__construct() while an XPath evaluation is in progress
61+
string(2) "nr"
62+
Error: Cannot call DOMXPath::__construct() while an XPath evaluation is in progress
63+
int(1)
64+
string(5) "other"
65+
Error: Cannot call Dom\XPath::__construct() while an XPath evaluation is in progress
66+
string(1) "r"
67+
string(1) "1"
68+
nested: 2
69+
Error: Cannot call Dom\XPath::__construct() while an XPath evaluation is in progress
70+
string(2) "nr"
71+
Error: Cannot call Dom\XPath::__construct() while an XPath evaluation is in progress
72+
int(1)
73+
string(5) "other"

‎ext/dom/xpath.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ static void dom_xpath_construct(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry *
126126
RETURN_THROWS();
127127
}
128128

129+
dom_xpath_object *intern = Z_XPATHOBJ_P(ZEND_THIS);
130+
if (UNEXPECTED(intern->evaluation_depth > 0)) {
131+
zend_throw_error(NULL, "Cannot call %s::__construct() while an XPath evaluation is in progress",
132+
ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
133+
RETURN_THROWS();
134+
}
135+
129136
DOM_GET_OBJ(docp, doc, xmlDocPtr, docobj);
130137

131138
xmlXPathContextPtr ctx = xmlXPathNewContext(docp);
@@ -134,7 +141,6 @@ static void dom_xpath_construct(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry *
134141
RETURN_THROWS();
135142
}
136143

137-
dom_xpath_object *intern = Z_XPATHOBJ_P(ZEND_THIS);
138144
xmlXPathContextPtr oldctx = intern->dom.ptr;
139145
if (oldctx != NULL) {
140146
php_libxml_decrement_doc_ref((php_libxml_node_object *) &intern->dom);
@@ -301,7 +307,9 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern)
301307
ctxp->nsNr = in_scope_ns.count;
302308
}
303309

310+
intern->evaluation_depth++;
304311
xmlXPathObjectPtr xpathobjp = xmlXPathEvalExpression(BAD_CAST expr, ctxp);
312+
intern->evaluation_depth--;
305313
ctxp->node = NULL;
306314

307315
if (register_node_ns && nodep != NULL) {

0 commit comments

Comments
 (0)