Skip to content

Commit 0ff6c54

Browse files
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.
1 parent efe4e6d commit 0ff6c54

File tree

2 files changed

+101
-2
lines changed

2 files changed

+101
-2
lines changed

ext/dom/parentnode/tree.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ zend_result dom_parent_node_child_element_count(dom_object *obj, zval *retval)
9898
}
9999
/* }}} */
100100

101+
static ZEND_COLD void dom_cannot_create_temp_nodes(void)
102+
{
103+
php_dom_throw_error_with_message(INVALID_MODIFICATION_ERR, "Unable to allocate temporary nodes", /* strict */ true);
104+
}
105+
101106
static bool dom_is_node_in_list(const zval *nodes, uint32_t nodesc, const xmlNode *node_to_find)
102107
{
103108
for (uint32_t i = 0; i < nodesc; i++) {
@@ -363,12 +368,17 @@ xmlNode* dom_zvals_to_single_node(php_libxml_ref_obj *document, xmlNode *context
363368
return dom_object_get_node(Z_DOMOBJ_P(nodes));
364369
} else {
365370
ZEND_ASSERT(Z_TYPE_P(nodes) == IS_STRING);
366-
return xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes));
371+
node = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes));
372+
if (UNEXPECTED(node == NULL)) {
373+
dom_cannot_create_temp_nodes();
374+
}
375+
return node;
367376
}
368377
}
369378

370379
node = xmlNewDocFragment(documentNode);
371380
if (UNEXPECTED(!node)) {
381+
dom_cannot_create_temp_nodes();
372382
return NULL;
373383
}
374384

@@ -408,6 +418,10 @@ xmlNode* dom_zvals_to_single_node(php_libxml_ref_obj *document, xmlNode *context
408418

409419
/* Text nodes can't violate the hierarchy at this point. */
410420
newNode = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL(nodes[i]), Z_STRLEN(nodes[i]));
421+
if (UNEXPECTED(newNode == NULL)) {
422+
dom_cannot_create_temp_nodes();
423+
goto err;
424+
}
411425
dom_add_child_without_merging(node, newNode);
412426
}
413427
}
@@ -434,7 +448,12 @@ static zend_result dom_sanity_check_node_list_types(zval *nodes, uint32_t nodesc
434448
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]));
435449
return FAILURE;
436450
}
437-
} else if (type != IS_STRING) {
451+
} else if (type == IS_STRING) {
452+
if (Z_STRLEN(nodes[i]) > INT_MAX) {
453+
zend_argument_value_error(i + 1, "must be less than or equal to %d bytes long", INT_MAX);
454+
return FAILURE;
455+
}
456+
} else {
438457
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]));
439458
return FAILURE;
440459
}
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)