Skip to content

Commit 42908f9

Browse files
committed
Fix GH-14702: DOMDocument::xinclude() crash
The xinclude code from libxml removes the fallback node, but the fallback node is still reference via $fallback. The solution is to detach the nodes that are going to be removed in advance. Closes GH-14704.
1 parent 056bec7 commit 42908f9

File tree

3 files changed

+123
-0
lines changed

3 files changed

+123
-0
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ PHP NEWS
99
. Fixed bug GH-14590 (Memory leak in FPM test gh13563-conf-bool-env.phpt.
1010
(nielsdos)
1111

12+
- Dom:
13+
. Fixed bug GH-14702 (DOMDocument::xinclude() crash). (nielsdos)
14+
1215
- Phar:
1316
. Fixed bug GH-14603 (null string from zip entry).
1417
(David Carlier)

ext/dom/document.c

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,58 @@ static void php_dom_remove_xinclude_nodes(xmlNodePtr cur) /* {{{ */
15661566
}
15671567
/* }}} */
15681568

1569+
/* Backported from master branch xml_common.h */
1570+
static zend_always_inline xmlNodePtr php_dom_next_in_tree_order(const xmlNode *nodep, const xmlNode *basep)
1571+
{
1572+
if (nodep->type == XML_ELEMENT_NODE && nodep->children) {
1573+
return nodep->children;
1574+
}
1575+
1576+
if (nodep->next) {
1577+
return nodep->next;
1578+
} else {
1579+
/* Go upwards, until we find a parent node with a next sibling, or until we hit the base. */
1580+
do {
1581+
nodep = nodep->parent;
1582+
if (nodep == basep) {
1583+
return NULL;
1584+
}
1585+
} while (nodep->next == NULL);
1586+
return nodep->next;
1587+
}
1588+
}
1589+
1590+
static void dom_xinclude_strip_references(xmlNodePtr basep)
1591+
{
1592+
php_libxml_node_free_resource(basep);
1593+
1594+
xmlNodePtr current = basep->children;
1595+
1596+
while (current) {
1597+
php_libxml_node_free_resource(current);
1598+
current = php_dom_next_in_tree_order(current, basep);
1599+
}
1600+
}
1601+
1602+
/* See GH-14702.
1603+
* We have to remove userland references to xinclude fallback nodes because libxml2 will make clones of these
1604+
* and remove the original nodes. If the originals are removed while there are still userland references
1605+
* this will cause memory corruption. */
1606+
static void dom_xinclude_strip_fallback_references(const xmlNode *basep)
1607+
{
1608+
xmlNodePtr current = basep->children;
1609+
1610+
while (current) {
1611+
if (current->type == XML_ELEMENT_NODE && current->ns != NULL && current->_private != NULL
1612+
&& xmlStrEqual(current->name, XINCLUDE_FALLBACK)
1613+
&& (xmlStrEqual(current->ns->href, XINCLUDE_NS) || xmlStrEqual(current->ns->href, XINCLUDE_OLD_NS))) {
1614+
dom_xinclude_strip_references(current);
1615+
}
1616+
1617+
current = php_dom_next_in_tree_order(current, basep);
1618+
}
1619+
}
1620+
15691621
/* {{{ Substitutues xincludes in a DomDocument */
15701622
PHP_METHOD(DOMDocument, xinclude)
15711623
{
@@ -1588,6 +1640,8 @@ PHP_METHOD(DOMDocument, xinclude)
15881640

15891641
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
15901642

1643+
dom_xinclude_strip_fallback_references((const xmlNode *) docp);
1644+
15911645
PHP_LIBXML_SANITIZE_GLOBALS(xinclude);
15921646
err = xmlXIncludeProcessFlags(docp, (int)flags);
15931647
PHP_LIBXML_RESTORE_GLOBALS(xinclude);

ext/dom/tests/gh14702.phpt

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
GH-14702 (DOMDocument::xinclude() crash)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$doc = new DOMDocument();
8+
$doc->loadXML(<<<XML
9+
<?xml version="1.0"?>
10+
<root>
11+
<child/>
12+
<include href="foo" xmlns="http://www.w3.org/2001/XInclude">
13+
<fallback/>
14+
</include>
15+
<keep/>
16+
</root>
17+
XML);
18+
$xi = $doc->createElementNS('http://www.w3.org/2001/XInclude', 'xi:include');
19+
$xi->setAttribute('href', 'nonexistent');
20+
21+
$fallback = $doc->createElementNS('http://www.w3.org/2001/XInclude', 'xi:fallback');
22+
$xi->appendChild($fallback);
23+
$child1 = $fallback->appendChild($doc->createElement('fallback-child1'));
24+
$child2 = $fallback->appendChild($doc->createElement('fallback-child2'));
25+
26+
$xpath = new DOMXPath($doc);
27+
$toReplace = $xpath->query('//child')->item(0);
28+
$toReplace->parentNode->replaceChild($xi, $toReplace);
29+
30+
$keep = $doc->documentElement->lastElementChild;
31+
32+
var_dump(@$doc->xinclude());
33+
echo $doc->saveXML();
34+
35+
var_dump($child1, $child2, $fallback, $keep->nodeName);
36+
37+
$keep->textContent = 'still works';
38+
echo $doc->saveXML();
39+
?>
40+
--EXPECT--
41+
int(2)
42+
<?xml version="1.0"?>
43+
<root>
44+
<fallback-child1/><fallback-child2/>
45+
46+
<keep/>
47+
</root>
48+
object(DOMElement)#4 (1) {
49+
["schemaTypeInfo"]=>
50+
NULL
51+
}
52+
object(DOMElement)#5 (1) {
53+
["schemaTypeInfo"]=>
54+
NULL
55+
}
56+
object(DOMElement)#3 (1) {
57+
["schemaTypeInfo"]=>
58+
NULL
59+
}
60+
string(4) "keep"
61+
<?xml version="1.0"?>
62+
<root>
63+
<fallback-child1/><fallback-child2/>
64+
65+
<keep>still works</keep>
66+
</root>

0 commit comments

Comments
 (0)