From 374bb2078357c94489e1581226dd737aaa02ef65 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 5 Aug 2026 09:16:12 -0400 Subject: [PATCH] Fix SimpleXML integer offsets that cannot resolve aliasing a node sxe_get_element_by_offset scanned with nodendx <= offset, so a negative offset skipped the loop and returned the node it started from, and the SXE_ITER_NONE branches of the read and write handlers aliased the node for every offset other than 0. Reads and isset() reported an existing element and writes overwrote it. An offset that resolves to no element now warns and leaves the document alone. Closes GH-23068 --- NEWS | 4 + ext/simplexml/simplexml.c | 38 +++++--- .../tests/sxe_int_offset_no_alias.phpt | 94 +++++++++++++++++++ 3 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 ext/simplexml/tests/sxe_int_offset_no_alias.phpt diff --git a/NEWS b/NEWS index 474db936ef15..d6b609698de9 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,10 @@ PHP NEWS . Fixed bug GH-23043 (broken session id code can cause zend_mm_heap corrupted). (ndossche) +- SimpleXML: + . Fixed integer element offsets that cannot resolve aliasing an existing + element. (iliaal) + - Sockets: . Fixed various memory related issues in ext/sockets. (David Carlier) diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 8cf8e657e58f..f3c1a073fcaf 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -134,7 +134,7 @@ static xmlNodePtr sxe_get_element_by_offset(php_sxe_object *sxe, zend_long offse return NULL; } } - while (node && nodendx <= offset) { + while (node && (offset < 0 || nodendx <= offset)) { if (node->type == XML_ELEMENT_NODE && match_ns(node, sxe->iter.nsprefix, sxe->iter.isprefix)) { if (sxe->iter.type == SXE_ITER_CHILD || ( sxe->iter.type == SXE_ITER_ELEMENT && xmlStrEqual(node->name, BAD_CAST ZSTR_VAL(sxe->iter.name)))) { @@ -302,14 +302,16 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements, } if (!member || Z_TYPE_P(member) == IS_LONG) { zend_long cnt = 0; + bool appendable = true; xmlNodePtr mynode = node; if (sxe->iter.type == SXE_ITER_CHILD) { node = php_sxe_get_first_node_non_destructive(sxe, node); } if (sxe->iter.type == SXE_ITER_NONE) { - if (member && Z_LVAL_P(member) > 0) { - php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only 0 such elements exist", mynode->name, Z_LVAL_P(member)); + if (member && Z_LVAL_P(member) != 0) { + node = NULL; + appendable = false; } } else if (member) { node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt); @@ -319,11 +321,13 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements, if (node) { node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix); } else if (type == BP_VAR_W || type == BP_VAR_RW) { - if (member && cnt < Z_LVAL_P(member)) { + if (member && (Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) { php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only " ZEND_LONG_FMT " such elements exist", mynode->name, Z_LVAL_P(member), cnt); } - node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL); - node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix); + if (appendable && (!member || Z_LVAL_P(member) >= 0)) { + node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL); + node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix); + } } } else { /* In BP_VAR_IS mode only return a proper node if it actually exists. */ @@ -527,19 +531,18 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value, if (!member || Z_TYPE_P(member) == IS_LONG) { if (node->type == XML_ATTRIBUTE_NODE) { zend_throw_error(NULL, "Cannot create duplicate attribute"); - if (value_str) { - zend_string_release(value_str); - } - return &EG(error_zval); + value = &EG(error_zval); + goto out; } if (sxe->iter.type == SXE_ITER_NONE) { - newnode = node; - ++counter; - if (member && Z_LVAL_P(member) > 0) { + if (member && Z_LVAL_P(member) != 0) { php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only 0 such elements exist", mynode->name, Z_LVAL_P(member)); value = &EG(error_zval); + goto out; } + newnode = node; + ++counter; } else if (member) { newnode = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt); if (newnode) { @@ -586,10 +589,14 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value, newnode = xmlNewTextChild(mynode, NULL, (xmlChar *)Z_STRVAL_P(member), value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL); } } else if (!member || Z_TYPE_P(member) == IS_LONG) { - if (member && cnt < Z_LVAL_P(member)) { + if (member && (Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) { php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only " ZEND_LONG_FMT " such elements exist", mynode->name, Z_LVAL_P(member), cnt); } - newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL); + if (member && Z_LVAL_P(member) < 0) { + value = &EG(error_zval); + } else { + newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL); + } } } else if (attribs) { if (Z_TYPE_P(member) == IS_LONG) { @@ -600,6 +607,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value, } } +out: if (member == &tmp_zv) { zval_ptr_dtor_str(&tmp_zv); } diff --git a/ext/simplexml/tests/sxe_int_offset_no_alias.phpt b/ext/simplexml/tests/sxe_int_offset_no_alias.phpt new file mode 100644 index 000000000000..03e10e7a6c94 --- /dev/null +++ b/ext/simplexml/tests/sxe_int_offset_no_alias.phpt @@ -0,0 +1,94 @@ +--TEST-- +Integer offsets that cannot resolve must never alias or mutate a node +--EXTENSIONS-- +simplexml +--FILE-- +abc'); +} + +function state(SimpleXMLElement $x): string { + return trim(strstr($x->asXML(), 'item[-1])); +var_dump($x->item[-1]); +var_dump((string) $x->item[0]); +$x->item[-1] = 'Z'; +echo state($x), "\n"; +unset($x->item[-1]); +echo state($x), "\n"; +$x->item[5] = 'P'; +echo state($x), "\n"; + +echo "== single element ==\n"; +$x = fresh(); +$n = $x->item[0]; +var_dump(isset($n[-1])); +var_dump($n[-1]); +var_dump($n[5]); +$n[-1] = 'Z'; +echo state($x), "\n"; +$n[5] = 'Y'; +echo state($x), "\n"; +unset($n[-1]); +echo state($x), "\n"; +var_dump((string) $n[0]); + +echo "== nested write ==\n"; +$x = fresh(); +try { + $x->item[-1]->kid = 'K'; +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), "\n"; +} +echo state($x), "\n"; + +echo "== attributes ==\n"; +$x = fresh(); +$at = $x->attributes(); +var_dump(isset($at[-1])); +var_dump($at[-1]); +$at[-1] = 'z'; +echo state($x), "\n"; +?> +--EXPECTF-- +== element list == +bool(false) +NULL +string(1) "a" + +Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d +abc +abc + +Warning: main(): Cannot add element item number 5 when only 3 such elements exist in %s on line %d +abcP +== single element == +bool(false) +NULL +NULL + +Warning: main(): Cannot add element item number -1 when only 0 such elements exist in %s on line %d +abc + +Warning: main(): Cannot add element item number 5 when only 0 such elements exist in %s on line %d +abc +abc +string(1) "a" +== nested write == + +Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d + +Notice: Indirect modification of overloaded element of SimpleXMLElement has no effect in %s on line %d +Error: Attempt to assign property "kid" on null +abc +== attributes == +bool(false) +NULL + +Warning: main(): Cannot change attribute number -1 when only 0 attributes exist in %s on line %d +abc