Skip to content

Commit 4595a57

Browse files
dstogovdstogov
committed
Fix clobering of operand by error handler in assignment to string offset (optimization and JIT support)
1 parent 09547c6 commit 4595a57

File tree

2 files changed

+87
-34
lines changed

2 files changed

+87
-34
lines changed

‎Zend/zend_execute.c

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,11 +1547,9 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
15471547
} else {
15481548
/* The string may be destroyed while throwing the notice.
15491549
* Temporarily increase the refcount to detect this situation. */
1550-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE)) {
1551-
GC_ADDREF(s);
1552-
}
1550+
GC_ADDREF(s);
15531551
offset = zend_check_string_offset(dim, BP_VAR_W EXECUTE_DATA_CC);
1554-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE) && GC_DELREF(s) == 0) {
1552+
if (GC_DELREF(s) == 0) {
15551553
zend_string_efree(s);
15561554
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
15571555
ZVAL_NULL(EX_VAR(opline->result.var));
@@ -1581,17 +1579,17 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
15811579
}
15821580

15831581
if (UNEXPECTED(Z_TYPE_P(value) != IS_STRING)) {
1582+
zend_string *tmp;
1583+
15841584
/* The string may be destroyed while throwing the notice.
15851585
* Temporarily increase the refcount to detect this situation. */
1586-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE)) {
1587-
GC_ADDREF(s);
1588-
}
1586+
GC_ADDREF(s);
15891587
if (UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
15901588
zval_undefined_cv((opline+1)->op1.var EXECUTE_DATA_CC);
15911589
}
15921590
/* Convert to string, just the time to pick the 1st byte */
1593-
zend_string *tmp = zval_try_get_string_func(value);
1594-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE) && GC_DELREF(s) == 0) {
1591+
tmp = zval_try_get_string_func(value);
1592+
if (GC_DELREF(s) == 0) {
15951593
zend_string_efree(s);
15961594
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
15971595
ZVAL_NULL(EX_VAR(opline->result.var));
@@ -1625,11 +1623,9 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
16251623

16261624
/* The string may be destroyed while throwing the notice.
16271625
* Temporarily increase the refcount to detect this situation. */
1628-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE)) {
1629-
GC_ADDREF(s);
1630-
}
1626+
GC_ADDREF(s);
16311627
zend_error(E_WARNING, "Only the first byte will be assigned to the string offset");
1632-
if (!(GC_FLAGS(s) & IS_ARRAY_IMMUTABLE) && GC_DELREF(s) == 0) {
1628+
if (GC_DELREF(s) == 0) {
16331629
zend_string_efree(s);
16341630
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
16351631
ZVAL_NULL(EX_VAR(opline->result.var));

‎ext/opcache/jit/zend_jit_helpers.c

Lines changed: 78 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,13 +1125,32 @@ static zend_never_inline ZEND_COLD void zend_wrong_string_offset(void)
11251125

11261126
static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim, zval *value, zval *result)
11271127
{
1128-
zend_string *old_str;
11291128
zend_uchar c;
11301129
size_t string_len;
11311130
zend_long offset;
1131+
zend_string *s;
1132+
1133+
/* separate string */
1134+
if (Z_REFCOUNTED_P(str) && Z_REFCOUNT_P(str) == 1) {
1135+
s = Z_STR_P(str);
1136+
} else {
1137+
s = zend_string_init(Z_STRVAL_P(str), Z_STRLEN_P(str), 0);
1138+
ZSTR_H(s) = ZSTR_H(Z_STR_P(str));
1139+
ZVAL_NEW_STR(str, s);
1140+
}
11321141

11331142
if (UNEXPECTED(Z_TYPE_P(dim) != IS_LONG)) {
1143+
/* The string may be destroyed while throwing the notice.
1144+
* Temporarily increase the refcount to detect this situation. */
1145+
GC_ADDREF(s);
11341146
offset = zend_check_string_offset(dim/*, BP_VAR_W*/);
1147+
if (GC_DELREF(s) == 0) {
1148+
zend_string_efree(s);
1149+
if (result) {
1150+
ZVAL_NULL(result);
1151+
}
1152+
return;
1153+
}
11351154
if (UNEXPECTED(EG(exception) != NULL)) {
11361155
if (UNEXPECTED(result)) {
11371156
ZVAL_UNDEF(result);
@@ -1141,7 +1160,7 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
11411160
} else {
11421161
offset = Z_LVAL_P(dim);
11431162
}
1144-
if (offset < -(zend_long)Z_STRLEN_P(str)) {
1163+
if (offset < -(zend_long)ZSTR_LEN(s)) {
11451164
/* Error on negative offset */
11461165
zend_error(E_WARNING, "Illegal string offset " ZEND_LONG_FMT, offset);
11471166
if (result) {
@@ -1151,8 +1170,35 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
11511170
}
11521171

11531172
if (Z_TYPE_P(value) != IS_STRING) {
1173+
zend_string *tmp;
1174+
1175+
/* The string may be destroyed while throwing the notice.
1176+
* Temporarily increase the refcount to detect this situation. */
1177+
GC_ADDREF(s);
1178+
1179+
if (UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
1180+
const zend_op *op_data = EG(current_execute_data)->opline + 1;
1181+
ZEND_ASSERT(op_data->opcode == ZEND_OP_DATA && op_data->op1_type == IS_CV);
1182+
zend_jit_undefined_op_helper(op_data->op1.var);
1183+
value = &EG(uninitialized_zval);
1184+
}
1185+
11541186
/* Convert to string, just the time to pick the 1st byte */
1155-
zend_string *tmp = zval_try_get_string_func(value);
1187+
tmp = zval_try_get_string_func(value);
1188+
1189+
if (GC_DELREF(s) == 0) {
1190+
zend_string_efree(s);
1191+
if (result) {
1192+
ZVAL_NULL(result);
1193+
}
1194+
return;
1195+
}
1196+
if (UNEXPECTED(!tmp)) {
1197+
if (result) {
1198+
ZVAL_UNDEF(result);
1199+
}
1200+
return;
1201+
}
11561202

11571203
if (UNEXPECTED(!tmp)) {
11581204
if (result) {
@@ -1180,27 +1226,37 @@ static zend_never_inline void zend_assign_to_string_offset(zval *str, zval *dim,
11801226
return;
11811227
}
11821228

1229+
/* The string may be destroyed while throwing the notice.
1230+
* Temporarily increase the refcount to detect this situation. */
1231+
GC_ADDREF(s);
11831232
zend_error(E_WARNING, "Only the first byte will be assigned to the string offset");
1233+
if (GC_DELREF(s) == 0) {
1234+
zend_string_efree(s);
1235+
if (result) {
1236+
ZVAL_NULL(result);
1237+
}
1238+
return;
1239+
}
1240+
/* Illegal offset assignment */
1241+
if (UNEXPECTED(EG(exception) != NULL)) {
1242+
if (result) {
1243+
ZVAL_UNDEF(result);
1244+
}
1245+
return;
1246+
}
11841247
}
11851248

11861249
if (offset < 0) { /* Handle negative offset */
1187-
offset += (zend_long)Z_STRLEN_P(str);
1250+
offset += (zend_long)ZSTR_LEN(s);
11881251
}
11891252

1190-
if ((size_t)offset >= Z_STRLEN_P(str)) {
1253+
if ((size_t)offset >= ZSTR_LEN(s)) {
11911254
/* Extend string if needed */
1192-
zend_long old_len = Z_STRLEN_P(str);
1193-
Z_STR_P(str) = zend_string_extend(Z_STR_P(str), offset + 1, 0);
1194-
Z_TYPE_INFO_P(str) = IS_STRING_EX;
1255+
zend_long old_len = ZSTR_LEN(s);
1256+
ZVAL_NEW_STR(str, zend_string_extend(s, (size_t)offset + 1, 0));
11951257
memset(Z_STRVAL_P(str) + old_len, ' ', offset - old_len);
11961258
Z_STRVAL_P(str)[offset+1] = 0;
1197-
} else if (!Z_REFCOUNTED_P(str)) {
1198-
old_str = Z_STR_P(str);
1199-
Z_STR_P(str) = zend_string_init(Z_STRVAL_P(str), Z_STRLEN_P(str), 0);
1200-
Z_TYPE_INFO_P(str) = IS_STRING_EX;
1201-
zend_string_release(old_str);
12021259
} else {
1203-
SEPARATE_STRING(str);
12041260
zend_string_forget_hash_val(Z_STR_P(str));
12051261
}
12061262

@@ -1283,6 +1339,11 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_obj_rw_helper(zval *object_ptr, zva
12831339

12841340
static void ZEND_FASTCALL zend_jit_assign_dim_helper(zval *object_ptr, zval *dim, zval *value, zval *result)
12851341
{
1342+
if (EXPECTED(Z_TYPE_P(object_ptr) == IS_STRING) && EXPECTED(dim != NULL)) {
1343+
zend_assign_to_string_offset(object_ptr, dim, value, result);
1344+
return;
1345+
}
1346+
12861347
if (dim && UNEXPECTED(Z_TYPE_P(dim) == IS_UNDEF)) {
12871348
const zend_op *opline = EG(current_execute_data)->opline;
12881349
zend_jit_undefined_op_helper(opline->op2.var);
@@ -1307,13 +1368,9 @@ static void ZEND_FASTCALL zend_jit_assign_dim_helper(zval *object_ptr, zval *dim
13071368
}
13081369
}
13091370
} else if (EXPECTED(Z_TYPE_P(object_ptr) == IS_STRING)) {
1310-
if (!dim) {
1311-
zend_throw_error(NULL, "[] operator not supported for strings");
1312-
if (result) {
1313-
ZVAL_UNDEF(result);
1314-
}
1315-
} else {
1316-
zend_assign_to_string_offset(object_ptr, dim, value, result);
1371+
zend_throw_error(NULL, "[] operator not supported for strings");
1372+
if (result) {
1373+
ZVAL_UNDEF(result);
13171374
}
13181375
} else {
13191376
zend_throw_error(NULL, "Cannot use a scalar value as an array");

0 commit comments

Comments
 (0)