Skip to content

Commit dd57694

Browse files
committed
Fix phpGH-22570: stack overflow serializing a deeply nested Dom\XMLDocument
The new-DOM XML serializer recurses through dom_xml_serialization_algorithm() for every element child, so a document nested deeply enough overflows the C stack and crashes during saveXml() or innerHTML. Add a stack-limit check at the dispatcher, throwing an Error on overflow, mirroring bd724bd. Gate the "Could not save document" warning and the innerHTML not-well-formed exception with !EG(exception) so the thrown Error propagates cleanly instead of being accompanied by a warning or replaced with the wrong exception type. Fixes phpGH-22570 Closes phpGH-22576
1 parent bc3a7c7 commit dd57694

5 files changed

Lines changed: 64 additions & 2 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ PHP NEWS
55
- DBA:
66
. Fixed OOB read on malformed length field in dba flatfile handler. (alhudz)
77

8+
- DOM:
9+
. Fixed bug GH-22570 (Stack overflow when serializing a deeply nested
10+
Dom\XMLDocument). (iliaal)
11+
812
- Exif:
913
. Fixed bug GH-11020 (exif_read_data() emits a spurious "Illegal IFD size"
1014
warning when an IFD is not followed by a next-IFD offset). (Eyüp Can Akman)

ext/dom/document.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,9 @@ static void dom_document_save_xml(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry
16781678
}
16791679

16801680
if (!res) {
1681-
php_error_docref(NULL, E_WARNING, "Could not save document");
1681+
if (!EG(exception)) {
1682+
php_error_docref(NULL, E_WARNING, "Could not save document");
1683+
}
16821684
RETURN_FALSE;
16831685
} else {
16841686
RETURN_NEW_STR(res);

ext/dom/inner_html_mixin.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ zend_result dom_element_inner_html_read(dom_object *obj, zval *retval)
103103
}
104104
if (UNEXPECTED(status < 0)) {
105105
smart_str_free_ex(&str, false);
106-
php_dom_throw_error_with_message(SYNTAX_ERR, "The resulting XML serialization is not well-formed", true);
106+
if (!EG(exception)) {
107+
php_dom_throw_error_with_message(SYNTAX_ERR, "The resulting XML serialization is not well-formed", true);
108+
}
107109
return FAILURE;
108110
}
109111
ZVAL_STR(retval, smart_str_extract(&str));
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
GH-22570 (Stack overflow when serializing a deeply nested Dom\XMLDocument)
3+
--EXTENSIONS--
4+
dom
5+
--SKIPIF--
6+
<?php
7+
if (ini_get('zend.max_allowed_stack_size') === false) {
8+
die('skip No stack limit support');
9+
}
10+
if (getenv('SKIP_ASAN')) {
11+
die('skip ASAN needs different stack limit setting due to more stack space usage');
12+
}
13+
?>
14+
--INI--
15+
zend.max_allowed_stack_size=512K
16+
--FILE--
17+
<?php
18+
// Build via the DOM API, not the parser: libxml caps parse depth even with
19+
// LIBXML_PARSEHUGE on some platforms; the serializer recursion is the bug.
20+
$doc = Dom\XMLDocument::createEmpty();
21+
$node = $doc->appendChild($doc->createElement('root'));
22+
for ($i = 0; $i < 100000; $i++) {
23+
$node = $node->appendChild($doc->createElement('a'));
24+
}
25+
26+
try {
27+
$doc->saveXml();
28+
} catch (\Error $e) {
29+
echo "saveXml: ", $e::class, ": ", $e->getMessage(), "\n";
30+
}
31+
32+
try {
33+
$doc->documentElement->innerHTML;
34+
} catch (\Error $e) {
35+
echo "innerHTML: ", $e::class, ": ", $e->getMessage(), "\n";
36+
}
37+
?>
38+
--EXPECT--
39+
saveXml: Error: Maximum call stack size reached. Infinite recursion?
40+
innerHTML: Error: Maximum call stack size reached. Infinite recursion?

ext/dom/xml_serializer.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,15 @@ static int dom_xml_serializing_a_document_node(
12501250
return 0;
12511251
}
12521252

1253+
static zend_always_inline bool dom_xml_serialize_check_stack_limit(void)
1254+
{
1255+
#ifdef ZEND_CHECK_STACK_LIMIT
1256+
return zend_call_stack_overflowed(EG(stack_limit));
1257+
#else
1258+
return false;
1259+
#endif
1260+
}
1261+
12531262
/* https://w3c.github.io/DOM-Parsing/#dfn-xml-serialization-algorithm */
12541263
static int dom_xml_serialization_algorithm(
12551264
dom_xml_serialize_ctx *ctx,
@@ -1261,6 +1270,11 @@ static int dom_xml_serialization_algorithm(
12611270
bool require_well_formed
12621271
)
12631272
{
1273+
if (UNEXPECTED(dom_xml_serialize_check_stack_limit())) {
1274+
zend_throw_error(NULL, "Maximum call stack size reached. Infinite recursion?");
1275+
return -1;
1276+
}
1277+
12641278
/* If node's interface is: */
12651279
switch (node->type) {
12661280
case XML_ELEMENT_NODE:

0 commit comments

Comments
 (0)