Skip to content

Commit 9435f4d

Browse files
nielsdosnielsdos
committed
Throw instead of silently failing when creating a too long text node
Lower branches suffer from this as well but we cannot change the behaviour there. We also add NULL checks to check for allocation failure. Closes GH-15014.
1 parent c4e1f2b commit 9435f4d

File tree

3 files changed

+103
-2
lines changed

3 files changed

+103
-2
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ PHP NEWS
1111

1212
- DOM:
1313
. Fix trampoline leak in xpath callables. (nielsdos)
14+
. Throw instead of silently failing when creating a too long text node in
15+
(DOM)ParentNode and (DOM)ChildNode. (nielsdos)
1416

1517
- OpenSSL:
1618
. Bumped minimum required OpenSSL version to 1.1.0. (cmb)

‎ext/dom/parentnode/tree.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ zend_result dom_parent_node_child_element_count(dom_object *obj, zval *retval)
8888
}
8989
/* }}} */
9090

91+
static ZEND_COLD void dom_cannot_create_temp_nodes(void)
92+
{
93+
php_dom_throw_error_with_message(INVALID_MODIFICATION_ERR, "Unable to allocate temporary nodes", /* strict */ true);
94+
}
95+
9196
static bool dom_is_node_in_list(const zval *nodes, uint32_t nodesc, const xmlNode *node_to_find)
9297
{
9398
for (uint32_t i = 0; i < nodesc; i++) {
@@ -353,12 +358,17 @@ xmlNode* dom_zvals_to_single_node(php_libxml_ref_obj *document, xmlNode *context
353358
return dom_object_get_node(Z_DOMOBJ_P(nodes));
354359
} else {
355360
ZEND_ASSERT(Z_TYPE_P(nodes) == IS_STRING);
356-
return xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes));
361+
node = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes));
362+
if (UNEXPECTED(node == NULL)) {
363+
dom_cannot_create_temp_nodes();
364+
}
365+
return node;
357366
}
358367
}
359368

360369
node = xmlNewDocFragment(documentNode);
361370
if (UNEXPECTED(!node)) {
371+
dom_cannot_create_temp_nodes();
362372
return NULL;
363373
}
364374

@@ -397,6 +407,10 @@ xmlNode* dom_zvals_to_single_node(php_libxml_ref_obj *document, xmlNode *context
397407

398408
/* Text nodes can't violate the hierarchy at this point. */
399409
newNode = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL(nodes[i]), Z_STRLEN(nodes[i]));
410+
if (UNEXPECTED(newNode == NULL)) {
411+
dom_cannot_create_temp_nodes();
412+
goto err;
413+
}
400414
dom_add_child_without_merging(node, newNode);
401415
}
402416
}
@@ -423,7 +437,12 @@ static zend_result dom_sanity_check_node_list_types(zval *nodes, uint32_t nodesc
423437
zend_argument_type_error(i + 1, "must be of type %s|string, %s given", ZSTR_VAL(node_ce->name), zend_zval_type_name(&nodes[i]));
424438
return FAILURE;
425439
}
426-
} else if (type != IS_STRING) {
440+
} else if (type == IS_STRING) {
441+
if (Z_STRLEN(nodes[i]) > INT_MAX) {
442+
zend_argument_value_error(i + 1, "must be less than or equal to %d bytes long", INT_MAX);
443+
return FAILURE;
444+
}
445+
} else {
427446
zend_argument_type_error(i + 1, "must be of type %s|string, %s given", ZSTR_VAL(node_ce->name), zend_zval_type_name(&nodes[i]));
428447
return FAILURE;
429448
}

‎ext/dom/tests/parentnode_childnode_too_long_text.phpt

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
--TEST--
2+
Passing a too long string to ChildNode or ParentNode methods causes an exception
3+
--EXTENSIONS--
4+
dom
5+
--INI--
6+
memory_limit=-1
7+
--SKIPIF--
8+
<?php
9+
if (PHP_INT_SIZE !== 8) die('skip Only for 64-bit');
10+
if (getenv('SKIP_SLOW_TESTS')) die('skip slow test');
11+
// Copied from file_get_contents_file_put_contents_5gb.phpt
12+
function get_system_memory(): int|float|false
13+
{
14+
if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
15+
// Windows-based memory check
16+
@exec('wmic OS get FreePhysicalMemory', $output);
17+
if (isset($output[1])) {
18+
return ((int)trim($output[1])) * 1024;
19+
}
20+
} else {
21+
// Unix/Linux-based memory check
22+
$memInfo = @file_get_contents("/proc/meminfo");
23+
if ($memInfo) {
24+
preg_match('/MemFree:\s+(\d+) kB/', $memInfo, $matches);
25+
return $matches[1] * 1024; // Convert to bytes
26+
}
27+
}
28+
return false;
29+
}
30+
if (get_system_memory() < 4 * 1024 * 1024 * 1024) {
31+
die('skip Reason: Insufficient RAM (less than 4GB)');
32+
}
33+
?>
34+
--FILE--
35+
<?php
36+
$dom = new DOMDocument;
37+
$element = $dom->appendChild($dom->createElement('root'));
38+
$str = str_repeat('X', 2**31 + 10);
39+
try {
40+
$element->append('x', $str);
41+
} catch (ValueError $e) {
42+
echo $e->getMessage(), "\n";
43+
}
44+
try {
45+
$element->prepend('x', $str);
46+
} catch (ValueError $e) {
47+
echo $e->getMessage(), "\n";
48+
}
49+
try {
50+
$element->after('x', $str);
51+
} catch (ValueError $e) {
52+
echo $e->getMessage(), "\n";
53+
}
54+
try {
55+
$element->before('x', $str);
56+
} catch (ValueError $e) {
57+
echo $e->getMessage(), "\n";
58+
}
59+
try {
60+
$element->replaceWith('x', $str);
61+
} catch (ValueError $e) {
62+
echo $e->getMessage(), "\n";
63+
}
64+
try {
65+
$element->replaceChildren('x', $str);
66+
} catch (ValueError $e) {
67+
echo $e->getMessage(), "\n";
68+
}
69+
var_dump($dom->childNodes->count());
70+
var_dump($element->childNodes->count());
71+
?>
72+
--EXPECT--
73+
DOMElement::append(): Argument #2 must be less than or equal to 2147483647 bytes long
74+
DOMElement::prepend(): Argument #2 must be less than or equal to 2147483647 bytes long
75+
DOMElement::after(): Argument #2 must be less than or equal to 2147483647 bytes long
76+
DOMElement::before(): Argument #2 must be less than or equal to 2147483647 bytes long
77+
DOMElement::replaceWith(): Argument #2 must be less than or equal to 2147483647 bytes long
78+
DOMElement::replaceChildren(): Argument #2 must be less than or equal to 2147483647 bytes long
79+
int(1)
80+
int(0)

0 commit comments

Comments
 (0)