From 0ff6c54eebb634531d18689b56e119deb52611d9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:41:29 +0200 Subject: [PATCH] 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. --- ext/dom/parentnode/tree.c | 23 +++++- .../parentnode_childnode_too_long_text.phpt | 80 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 ext/dom/tests/parentnode_childnode_too_long_text.phpt diff --git a/ext/dom/parentnode/tree.c b/ext/dom/parentnode/tree.c index f1cd4fd742ef4..43dda3c340a88 100644 --- a/ext/dom/parentnode/tree.c +++ b/ext/dom/parentnode/tree.c @@ -98,6 +98,11 @@ zend_result dom_parent_node_child_element_count(dom_object *obj, zval *retval) } /* }}} */ +static ZEND_COLD void dom_cannot_create_temp_nodes(void) +{ + php_dom_throw_error_with_message(INVALID_MODIFICATION_ERR, "Unable to allocate temporary nodes", /* strict */ true); +} + static bool dom_is_node_in_list(const zval *nodes, uint32_t nodesc, const xmlNode *node_to_find) { 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 return dom_object_get_node(Z_DOMOBJ_P(nodes)); } else { ZEND_ASSERT(Z_TYPE_P(nodes) == IS_STRING); - return xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes)); + node = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL_P(nodes), Z_STRLEN_P(nodes)); + if (UNEXPECTED(node == NULL)) { + dom_cannot_create_temp_nodes(); + } + return node; } } node = xmlNewDocFragment(documentNode); if (UNEXPECTED(!node)) { + dom_cannot_create_temp_nodes(); return NULL; } @@ -408,6 +418,10 @@ xmlNode* dom_zvals_to_single_node(php_libxml_ref_obj *document, xmlNode *context /* Text nodes can't violate the hierarchy at this point. */ newNode = xmlNewDocTextLen(documentNode, BAD_CAST Z_STRVAL(nodes[i]), Z_STRLEN(nodes[i])); + if (UNEXPECTED(newNode == NULL)) { + dom_cannot_create_temp_nodes(); + goto err; + } dom_add_child_without_merging(node, newNode); } } @@ -434,7 +448,12 @@ static zend_result dom_sanity_check_node_list_types(zval *nodes, uint32_t nodesc 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])); return FAILURE; } - } else if (type != IS_STRING) { + } else if (type == IS_STRING) { + if (Z_STRLEN(nodes[i]) > INT_MAX) { + zend_argument_value_error(i + 1, "must be less than or equal to %d bytes long", INT_MAX); + return FAILURE; + } + } else { 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])); return FAILURE; } diff --git a/ext/dom/tests/parentnode_childnode_too_long_text.phpt b/ext/dom/tests/parentnode_childnode_too_long_text.phpt new file mode 100644 index 0000000000000..62edaebfe4a88 --- /dev/null +++ b/ext/dom/tests/parentnode_childnode_too_long_text.phpt @@ -0,0 +1,80 @@ +--TEST-- +Passing a too long string to ChildNode or ParentNode methods causes an exception +--EXTENSIONS-- +dom +--INI-- +memory_limit=-1 +--SKIPIF-- + +--FILE-- +appendChild($dom->createElement('root')); +$str = str_repeat('X', 2**31 + 10); +try { + $element->append('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + $element->prepend('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + $element->after('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + $element->before('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + $element->replaceWith('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + $element->replaceChildren('x', $str); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom->childNodes->count()); +var_dump($element->childNodes->count()); +?> +--EXPECT-- +DOMElement::append(): Argument #2 must be less than or equal to 2147483647 bytes long +DOMElement::prepend(): Argument #2 must be less than or equal to 2147483647 bytes long +DOMElement::after(): Argument #2 must be less than or equal to 2147483647 bytes long +DOMElement::before(): Argument #2 must be less than or equal to 2147483647 bytes long +DOMElement::replaceWith(): Argument #2 must be less than or equal to 2147483647 bytes long +DOMElement::replaceChildren(): Argument #2 must be less than or equal to 2147483647 bytes long +int(1) +int(0)