Skip to content

Commit d09e5e2

Browse files
committed
Fix DOMNameSpaceNode clone UAF after xinclude
Clone built the fake namespace decl from original_node->parent, which xinclude has already freed; parent_intern is the durable handle. Closes GH-23248
1 parent d9c58ee commit d09e5e2

3 files changed

Lines changed: 56 additions & 6 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ PHP NEWS
66
. Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or
77
next() call on the inner generator). (iliaal)
88

9+
- DOM:
10+
. Fixed a use-after-free when cloning a DOMNameSpaceNode after
11+
DOMDocument::xinclude(). (iliaal)
12+
913
- Opcache:
1014
. Fixed opcache.protect_memory race under ZTS. (realFlowControl)
1115

ext/dom/php_dom.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static HashTable dom_xpath_prop_handlers;
146146

147147
static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type);
148148
static void dom_object_namespace_node_free_storage(zend_object *object);
149-
static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original);
149+
static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original, xmlDocPtr fallback_doc);
150150

151151
typedef zend_result (*dom_read_t)(dom_object *obj, zval *retval);
152152
typedef zend_result (*dom_write_t)(dom_object *obj, zval *newval);
@@ -705,7 +705,8 @@ static zend_object *dom_object_namespace_node_clone_obj(zend_object *zobject)
705705
xmlNodePtr original_node = dom_object_get_node(&intern->dom);
706706
if (original_node != NULL) {
707707
ZEND_ASSERT(original_node->type == XML_NAMESPACE_DECL);
708-
xmlNodePtr cloned_node = php_dom_create_fake_namespace_decl_node_ptr(original_node->parent, original_node->ns);
708+
xmlNodePtr parent = intern->parent_intern ? dom_object_get_node(intern->parent_intern) : NULL;
709+
xmlNodePtr cloned_node = php_dom_create_fake_namespace_decl_node_ptr(parent, original_node->ns, original_node->doc);
709710
dom_update_refcount_after_clone(&intern->dom, original_node, &clone_intern->dom, cloned_node);
710711
}
711712

@@ -2318,15 +2319,16 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) {
23182319
}
23192320
/* }}} end dom_get_nsdecl */
23202321

2321-
static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original)
2322+
static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original, xmlDocPtr fallback_doc)
23222323
{
23232324
xmlNodePtr attrp;
2325+
xmlDocPtr doc = nodep ? nodep->doc : fallback_doc;
23242326
xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL);
23252327
if (original->prefix) {
23262328
curns->prefix = xmlStrdup(original->prefix);
2327-
attrp = xmlNewDocNode(nodep->doc, NULL, BAD_CAST original->prefix, original->href);
2329+
attrp = xmlNewDocNode(doc, NULL, BAD_CAST original->prefix, original->href);
23282330
} else {
2329-
attrp = xmlNewDocNode(nodep->doc, NULL, BAD_CAST "xmlns", original->href);
2331+
attrp = xmlNewDocNode(doc, NULL, BAD_CAST "xmlns", original->href);
23302332
}
23312333
attrp->type = XML_NAMESPACE_DECL;
23322334
attrp->parent = nodep;
@@ -2337,7 +2339,7 @@ static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep,
23372339
/* Note: Assumes the additional lifetime was already added in the caller. */
23382340
xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern)
23392341
{
2340-
xmlNodePtr attrp = php_dom_create_fake_namespace_decl_node_ptr(nodep, original);
2342+
xmlNodePtr attrp = php_dom_create_fake_namespace_decl_node_ptr(nodep, original, NULL);
23412343
php_dom_create_object(attrp, return_value, parent_intern);
23422344
/* This object must exist, because we just created an object for it via php_dom_create_object(). */
23432345
php_dom_namespace_node_obj_from_obj(Z_OBJ_P(return_value))->parent_intern = parent_intern;
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
DOMNameSpaceNode clone after xinclude does not use a dangling parent
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$included = __DIR__ . '/dom_namespacenode_clone_xinclude_included.xml';
8+
file_put_contents($included, '<?xml version="1.0"?><included/>');
9+
$href = 'file:///' . ltrim(str_replace('\\', '/', $included), '/');
10+
11+
$doc = new DOMDocument();
12+
$doc->loadXML('<?xml version="1.0"?>
13+
<root xmlns:xi="http://www.w3.org/2001/XInclude">
14+
<xi:include href="' . $href . '" xmlns:local="urn:test"/>
15+
</root>');
16+
17+
$xpath = new DOMXPath($doc);
18+
$xpath->registerNamespace('xi', 'http://www.w3.org/2001/XInclude');
19+
$xi = $xpath->query('//xi:include')->item(0);
20+
$ns = $xpath->query('namespace::local', $xi)->item(0);
21+
22+
$live = clone $ns;
23+
echo "live clone: ", $live->nodeName, "\n";
24+
echo "live parent: ", $live->parentNode->nodeName, "\n";
25+
26+
$doc->xinclude();
27+
28+
$clone = clone $ns;
29+
echo "after xinclude: ", $clone->nodeName, "\n";
30+
var_dump($clone->parentNode);
31+
var_dump($clone->parentElement);
32+
var_dump($clone->isConnected);
33+
?>
34+
--CLEAN--
35+
<?php
36+
@unlink(__DIR__ . '/dom_namespacenode_clone_xinclude_included.xml');
37+
?>
38+
--EXPECT--
39+
live clone: xmlns:local
40+
live parent: xi:include
41+
after xinclude: xmlns:local
42+
NULL
43+
NULL
44+
bool(false)

0 commit comments

Comments
 (0)