Skip to content

Fix bug 71572 #1761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix bug 71572 #1761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Zend/tests/bug71572.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Bug #71572: String offset assignment from an empty string inserts null byte
--FILE--
<?php

$str = "abc";
var_dump($str{0} = "");
var_dump($str{1} = "");
var_dump($str{3} = "");
var_dump($str{10} = "");
var_dump($str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test case for $str{10} = "" as well. Currently this will still extend the string, and likely additionally leave one uninitialized byte.

?>
==DONE==
--EXPECTF--
Warning: Cannot assign an empty string to a string offset in %s on line %d
NULL

Warning: Cannot assign an empty string to a string offset in %s on line %d
NULL

Warning: Cannot assign an empty string to a string offset in %s on line %d
NULL

Warning: Cannot assign an empty string to a string offset in %s on line %d
NULL
string(3) "abc"
==DONE==
41 changes: 28 additions & 13 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1327,8 +1327,10 @@ static zend_never_inline void zend_binary_assign_op_obj_dim(zval *object, zval *
static void zend_assign_to_string_offset(zval *str, zend_long offset, zval *value, zval *result)
{
zend_string *old_str;
zend_uchar c;
size_t string_len;

if (offset < 0) {
if (offset < 0) { /* Error on negative offset */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to be a new line? Using a tab here is strange.

zend_error(E_WARNING, "Illegal string offset: " ZEND_LONG_FMT, offset);
zend_string_release(Z_STR_P(str));
if (result) {
Expand All @@ -1337,8 +1339,29 @@ static void zend_assign_to_string_offset(zval *str, zend_long offset, zval *valu
return;
}

if (Z_TYPE_P(value) != IS_STRING) {
/* Convert to string, just the time to pick the 1st byte */
zend_string *tmp = zval_get_string(value);

string_len = ZSTR_LEN(tmp);
c = (zend_uchar)ZSTR_VAL(tmp)[0];
zend_string_release(tmp);
} else {
string_len = Z_STRLEN_P(value);
c = (zend_uchar)Z_STRVAL_P(value)[0];
}

if (string_len == 0) { /* Error on empty input string */
zend_error(E_WARNING, "Cannot assign an empty string to a string offset");
zend_string_release(Z_STR_P(str));
if (result) {
ZVAL_NULL(result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure about this return value, we should cross check it with other assignment failure cases. I think we should be either returning an empty string here (the RHS) or the character at the modified offset of the string (the "new" LHS).

}
return;
}

old_str = Z_STR_P(str);
if ((size_t)offset >= Z_STRLEN_P(str)) {
if ((size_t)offset >= Z_STRLEN_P(str)) { /* Extend string if needed */
zend_long old_len = Z_STRLEN_P(str);
Z_STR_P(str) = zend_string_extend(Z_STR_P(str), offset + 1, 0);
Z_TYPE_INFO_P(str) = IS_STRING_EX;
Expand All @@ -1349,27 +1372,19 @@ static void zend_assign_to_string_offset(zval *str, zend_long offset, zval *valu
Z_TYPE_INFO_P(str) = IS_STRING_EX;
}

if (Z_TYPE_P(value) != IS_STRING) {
zend_string *tmp = zval_get_string(value);
Z_STRVAL_P(str)[offset] = c;

Z_STRVAL_P(str)[offset] = ZSTR_VAL(tmp)[0];
zend_string_release(tmp);
} else {
Z_STRVAL_P(str)[offset] = Z_STRVAL_P(value)[0];
}
/*
* the value of an assignment to a string offset is undefined
T(result->u.var).var = &T->str_offset.str;
*/

zend_string_release(old_str);
if (result) {
zend_uchar c = (zend_uchar)Z_STRVAL_P(str)[offset];

if (result) { /* Return the new character */
if (CG(one_char_string)[c]) {
ZVAL_INTERNED_STR(result, CG(one_char_string)[c]);
} else {
ZVAL_NEW_STR(result, zend_string_init(Z_STRVAL_P(str) + offset, 1, 0));
ZVAL_NEW_STR(result, zend_string_init((char *)(&c), 1, 0));
}
}
}
Expand Down