Skip to content

Commit cebf67e

Browse files
committed
Fix GH-20482: heap use-after-free in ZEND_ASSIGN_DIM via re-entrant user output handler
When ZEND_ASSIGN_DIM evaluates an undefined CV as the RHS, the warning emitted by zval_undefined_cv() can be routed through a user output handler (e.g. via ob_start with a small chunk size). The handler runs arbitrary PHP code, which may free, unset or rehash the array we are writing to, leaving the previously-fetched variable_ptr pointing into freed/relocated memory. The next zend_assign_to_variable_ex then performs a heap use-after-free. In the OP2_TYPE != IS_UNUSED branch: - Switch GET_OP_DATA_ZVAL_PTR to GET_OP_DATA_ZVAL_PTR_UNDEF so we no longer auto-emit the warning. Detect IS_UNDEF explicitly and call zval_undefined_cv around a temporary GC_ADDREF / GC_DELREF on the target HashTable, mirroring the IS_UNUSED branch and the previous fix in slow_index_convert (b594a95). - Reorder the pipeline: fetch the data value (and emit the warning) *before* resolving variable_ptr. Resolving the dim address only afterwards guarantees a fresh bucket pointer that cannot have been invalidated by reentrant user code freeing the array, rehashing it or unsetting keys. Includes phpt regressions covering the three reentrancy paths: - output handler reassigns the array, - output handler unsets the array, - output handler rehashes the array (would invalidate any pre-fetched bucket pointer).
1 parent 1462499 commit cebf67e

5 files changed

Lines changed: 1234 additions & 147 deletions

File tree

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ PHP NEWS
1818
initialization). (Arnaud)
1919
. Enabled the TAILCALL VM on Windows when compiling with Clang >= 19 x86_64.
2020
(henderkes)
21+
. Fixed bug GH-20482 (Heap use-after-free in ZEND_ASSIGN_DIM when a user
22+
output handler clobbers the array during the undefined-variable
23+
warning). (lacatoire)
2124

2225
- BCMath:
2326
. Added NUL-byte validation to BCMath functions. (jorgsowa)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-20482: heap use-after-free in ZEND_ASSIGN_DIM when an output handler clobbers the array during the undefined-variable warning
3+
--FILE--
4+
<?php
5+
$a = [1, 2, 3];
6+
7+
// Chunk size 1 forces the handler to run synchronously when the warning
8+
// emitted by the undefined RHS is buffered.
9+
ob_start(function () use (&$a) {
10+
// Drop the only reference to the original array, freeing it.
11+
$a = "freed";
12+
return '';
13+
}, 1);
14+
15+
// $undef is undefined: emitting the "Undefined variable" warning recurses
16+
// into the output handler above, which frees $a. Without the fix the next
17+
// store would write into the freed bucket.
18+
$a[0] = $undef;
19+
20+
ob_end_clean();
21+
var_dump($a);
22+
echo "ok\n";
23+
?>
24+
--EXPECT--
25+
string(5) "freed"
26+
ok
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-20482: ZEND_ASSIGN_DIM does not crash when the output handler mutates the array via a reference during the undefined-variable warning
3+
--FILE--
4+
<?php
5+
$a = ['existing' => 0];
6+
7+
ob_start(function () use (&$a) {
8+
// Mutate the array many times via the reference. Each nested
9+
// ZEND_ASSIGN_DIM observes the temporary refcount bump from the outer
10+
// handler and SEPARATEs the array, so the writes land in a duplicate;
11+
// the outer assignment is then aborted cleanly via assign_dim_error
12+
// when the original drops to refcount 0. The point of the test is
13+
// simply that the whole thing must not UAF or crash, regardless of
14+
// which array $a ends up pointing at.
15+
for ($i = 0; $i < 64; $i++) {
16+
$a["k$i"] = $i;
17+
}
18+
return '';
19+
}, 1);
20+
21+
$a['target'] = $undef;
22+
23+
ob_end_clean();
24+
var_dump(count($a) >= 65);
25+
echo "ok\n";
26+
?>
27+
--EXPECT--
28+
bool(true)
29+
ok

Zend/zend_vm_def.h

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,16 +2730,37 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
27302730
}
27312731
}
27322732
} else {
2733+
HashTable *ht = Z_ARRVAL_P(object_ptr);
27332734
dim = GET_OP2_ZVAL_PTR_UNDEF(BP_VAR_R);
2735+
/* Order is critical: fetch the data value (which may emit an
2736+
* "Undefined variable" warning that can recurse into user code
2737+
* via a user output handler) before resolving variable_ptr.
2738+
* Resolving variable_ptr only afterwards guarantees a fresh
2739+
* bucket pointer that cannot have been invalidated by user
2740+
* code freeing the array, rehashing it, or unsetting keys. */
2741+
value = GET_OP_DATA_ZVAL_PTR_UNDEF(BP_VAR_R);
2742+
if (OP_DATA_TYPE == IS_CV && UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
2743+
/* Temporarily addref the array around the warning so we
2744+
* can detect a full destruction (last ref dropped by
2745+
* reentrant user code) and bail out cleanly. Mirrors the
2746+
* IS_UNUSED branch above. */
2747+
if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
2748+
GC_ADDREF(ht);
2749+
}
2750+
value = zval_undefined_cv((opline+1)->op1.var EXECUTE_DATA_CC);
2751+
if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) && !GC_DELREF(ht)) {
2752+
zend_array_destroy(ht);
2753+
ZEND_VM_C_GOTO(assign_dim_error);
2754+
}
2755+
}
27342756
if (OP2_TYPE == IS_CONST) {
2735-
variable_ptr = zend_fetch_dimension_address_inner_W_CONST(Z_ARRVAL_P(object_ptr), dim EXECUTE_DATA_CC);
2757+
variable_ptr = zend_fetch_dimension_address_inner_W_CONST(ht, dim EXECUTE_DATA_CC);
27362758
} else {
2737-
variable_ptr = zend_fetch_dimension_address_inner_W(Z_ARRVAL_P(object_ptr), dim EXECUTE_DATA_CC);
2759+
variable_ptr = zend_fetch_dimension_address_inner_W(ht, dim EXECUTE_DATA_CC);
27382760
}
27392761
if (UNEXPECTED(variable_ptr == NULL)) {
27402762
ZEND_VM_C_GOTO(assign_dim_error);
27412763
}
2742-
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
27432764
value = zend_assign_to_variable_ex(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);
27442765
}
27452766
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {

0 commit comments

Comments
 (0)