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