Skip to content

Commit 7b68ff4

Browse files
committed
Revert "Fix GH-10168: heap-buffer-overflow at zval_undefined_cv"
This reverts commit 71ddede.
1 parent d9ac59b commit 7b68ff4

8 files changed

+193
-487
lines changed

NEWS

-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ PHP NEWS
1010
Generator emits an unavoidable fatal error or crashes). (Arnaud)
1111
. Fixed bug GH-10437 (Segfault/assertion when using fibers in shutdown
1212
function after bailout). (trowski)
13-
. Fixed bug GH-10168: use-after-free when utilizing assigned object freed
14-
during assignment. (nielsdos)
1513
. Fixed SSA object type update for compound assignment opcodes. (nielsdos)
1614

1715
- Curl:

Zend/tests/gh10168_1.phpt

-32
This file was deleted.

Zend/tests/gh10168_2.phpt

-32
This file was deleted.

Zend/tests/gh10168_3.phpt

-30
This file was deleted.

Zend/zend_execute.c

+1-9
Original file line numberDiff line numberDiff line change
@@ -3562,7 +3562,7 @@ static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) {
35623562
}
35633563
}
35643564

3565-
ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict, zval *result_variable_ptr)
3565+
ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict)
35663566
{
35673567
bool ret;
35683568
zval value;
@@ -3582,9 +3582,6 @@ ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *ori
35823582
} else {
35833583
zval_ptr_dtor_nogc(&value);
35843584
}
3585-
if (result_variable_ptr) {
3586-
ZVAL_COPY(result_variable_ptr, variable_ptr);
3587-
}
35883585
if (value_type & (IS_VAR|IS_TMP_VAR)) {
35893586
if (UNEXPECTED(ref)) {
35903587
if (UNEXPECTED(GC_DELREF(ref) == 0)) {
@@ -3598,11 +3595,6 @@ ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *ori
35983595
return variable_ptr;
35993596
}
36003597

3601-
ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict)
3602-
{
3603-
return zend_assign_to_typed_ref_and_result(variable_ptr, orig_value, value_type, strict, NULL);
3604-
}
3605-
36063598
ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
36073599
zval *val = orig_val;
36083600
if (Z_ISREF_P(val) && ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(val))) {

Zend/zend_execute.h

+9-41
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ ZEND_API bool zend_verify_internal_return_type(zend_function *zf, zval *ret);
108108
ZEND_API void ZEND_FASTCALL zend_ref_add_type_source(zend_property_info_source_list *source_list, zend_property_info *prop);
109109
ZEND_API void ZEND_FASTCALL zend_ref_del_type_source(zend_property_info_source_list *source_list, zend_property_info *prop);
110110

111-
ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict, zval *result_variable_ptr);
112111
ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict);
113112

114113
static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type)
@@ -138,22 +137,12 @@ static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *v
138137
}
139138
}
140139

141-
static zend_always_inline void zend_handle_garbage_from_variable_assignment(zend_refcounted *garbage)
142-
{
143-
if (GC_DELREF(garbage) == 0) {
144-
rc_dtor_func(garbage);
145-
} else { /* we need to split */
146-
/* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */
147-
if (UNEXPECTED(GC_MAY_LEAK(garbage))) {
148-
gc_possible_root(garbage);
149-
}
150-
}
151-
}
152-
153140
static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
154141
{
155142
do {
156143
if (UNEXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
144+
zend_refcounted *garbage;
145+
157146
if (Z_ISREF_P(variable_ptr)) {
158147
if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(variable_ptr)))) {
159148
return zend_assign_to_typed_ref(variable_ptr, value, value_type, strict);
@@ -164,42 +153,21 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval
164153
break;
165154
}
166155
}
167-
zend_refcounted *garbage = Z_COUNTED_P(variable_ptr);
156+
garbage = Z_COUNTED_P(variable_ptr);
168157
zend_copy_to_variable(variable_ptr, value, value_type);
169-
zend_handle_garbage_from_variable_assignment(garbage);
170-
return variable_ptr;
171-
}
172-
} while (0);
173-
174-
zend_copy_to_variable(variable_ptr, value, value_type);
175-
return variable_ptr;
176-
}
177-
178-
static zend_always_inline zval* zend_assign_to_two_variables(zval *result_variable_ptr, zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
179-
{
180-
do {
181-
if (UNEXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
182-
if (Z_ISREF_P(variable_ptr)) {
183-
if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(variable_ptr)))) {
184-
variable_ptr = zend_assign_to_typed_ref_and_result(variable_ptr, value, value_type, strict, result_variable_ptr);
185-
return variable_ptr;
186-
}
187-
188-
variable_ptr = Z_REFVAL_P(variable_ptr);
189-
if (EXPECTED(!Z_REFCOUNTED_P(variable_ptr))) {
190-
break;
158+
if (GC_DELREF(garbage) == 0) {
159+
rc_dtor_func(garbage);
160+
} else { /* we need to split */
161+
/* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */
162+
if (UNEXPECTED(GC_MAY_LEAK(garbage))) {
163+
gc_possible_root(garbage);
191164
}
192165
}
193-
zend_refcounted *garbage = Z_COUNTED_P(variable_ptr);
194-
zend_copy_to_variable(variable_ptr, value, value_type);
195-
ZVAL_COPY(result_variable_ptr, variable_ptr);
196-
zend_handle_garbage_from_variable_assignment(garbage);
197166
return variable_ptr;
198167
}
199168
} while (0);
200169

201170
zend_copy_to_variable(variable_ptr, value, value_type);
202-
ZVAL_COPY(result_variable_ptr, variable_ptr);
203171
return variable_ptr;
204172
}
205173

Zend/zend_vm_def.h

+7-13
Original file line numberDiff line numberDiff line change
@@ -2577,9 +2577,6 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
25772577
Z_ADDREF_P(value);
25782578
}
25792579
}
2580-
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
2581-
ZVAL_COPY(EX_VAR(opline->result.var), value);
2582-
}
25832580
} else {
25842581
dim = GET_OP2_ZVAL_PTR_UNDEF(BP_VAR_R);
25852582
if (OP2_TYPE == IS_CONST) {
@@ -2591,11 +2588,10 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
25912588
ZEND_VM_C_GOTO(assign_dim_error);
25922589
}
25932590
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
2594-
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
2595-
zend_assign_to_two_variables(EX_VAR(opline->result.var), variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
2596-
} else {
2597-
zend_assign_to_variable(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
2598-
}
2591+
value = zend_assign_to_variable(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
2592+
}
2593+
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
2594+
ZVAL_COPY(EX_VAR(opline->result.var), value);
25992595
}
26002596
} else {
26012597
if (EXPECTED(Z_ISREF_P(object_ptr))) {
@@ -2689,14 +2685,12 @@ ZEND_VM_HANDLER(22, ZEND_ASSIGN, VAR|CV, CONST|TMP|VAR|CV, SPEC(RETVAL))
26892685
value = GET_OP2_ZVAL_PTR(BP_VAR_R);
26902686
variable_ptr = GET_OP1_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
26912687

2688+
value = zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
26922689
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
2693-
zend_assign_to_two_variables(EX_VAR(opline->result.var), variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
2694-
} else {
2695-
zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
2690+
ZVAL_COPY(EX_VAR(opline->result.var), value);
26962691
}
2697-
26982692
FREE_OP1_VAR_PTR();
2699-
/* zend_assign_to_(two_)variable(s)() always takes care of op2, never free it! */
2693+
/* zend_assign_to_variable() always takes care of op2, never free it! */
27002694

27012695
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
27022696
}

0 commit comments

Comments
 (0)