Skip to content

Commit 59de8a8

Browse files
ndosschearnaud-lb
andcommitted
Fix escape analysis for objects implementing ArrayAccess
Objects that implement ArrayAccess can run arbitrayr code on isset(), dimensions accesses etc. SCCP can then run on stale property values that were manipulated by such ArrayAccess handlers. We also take into accuont that we don't know the interfaces yet of unlinked classes. Closes GH-24089. Co-authored-by: Arnaud Le Blanc <arnaud.lb@gmail.com>
1 parent 55e44ee commit 59de8a8

3 files changed

Lines changed: 104 additions & 4 deletions

File tree

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ PHP NEWS
1818
. Fix throwing behaviour with recursive arrays in VM. (ndossche)
1919
. Fixed skipped catch block when switching fibers during exception unwinding
2020
(Nicolas Grekas)
21+
. Fix escape analysis for objects implementing ArrayAccess. (ndossche, Arnaud)
2122

2223
- DOM:
2324
. Fixed bug GH-23352 (UAF reading an attribute value node retained across

‎Zend/Optimizer/escape_analysis.c‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,25 @@ static bool is_local_def(zend_op_array *op_array, zend_ssa *ssa, int def, int va
272272
}
273273
/* }}} */
274274

275+
/* ArrayAccess methods receive the object, so dimension accesses on it let it escape. */
276+
static bool may_be_array_access(const zend_ssa_var_info *info) /* {{{ */
277+
{
278+
if (!(info->type & MAY_BE_OBJECT)) {
279+
return 0;
280+
}
281+
282+
const zend_class_entry *ce = info->ce;
283+
if (!ce || info->is_instanceof) {
284+
return 1;
285+
}
286+
if (ce->ce_flags & ZEND_ACC_LINKED) {
287+
return ce->arrayaccess_funcs_ptr != NULL;
288+
}
289+
/* The interfaces of unlinked classes are not known yet. */
290+
return ce->num_interfaces || ce->parent_name;
291+
}
292+
/* }}} */
293+
275294
static bool is_escape_use(zend_op_array *op_array, zend_ssa *ssa, int use, int var) /* {{{ */
276295
{
277296
zend_ssa_op *ssa_op = ssa->ops + use;
@@ -291,18 +310,22 @@ static bool is_escape_use(zend_op_array *op_array, zend_ssa *ssa, int use, int v
291310
}
292311
break;
293312
case ZEND_ISSET_ISEMPTY_DIM_OBJ:
294-
case ZEND_ISSET_ISEMPTY_PROP_OBJ:
295313
case ZEND_FETCH_DIM_R:
296-
case ZEND_FETCH_OBJ_R:
297314
case ZEND_FETCH_DIM_IS:
315+
case ZEND_ASSIGN_DIM_OP:
316+
case ZEND_ASSIGN_DIM:
317+
if (may_be_array_access(&ssa->var_info[ssa_op->op1_use])) {
318+
return 1;
319+
}
320+
break;
321+
case ZEND_ISSET_ISEMPTY_PROP_OBJ:
322+
case ZEND_FETCH_OBJ_R:
298323
case ZEND_FETCH_OBJ_IS:
299324
break;
300325
case ZEND_ASSIGN_OP:
301326
return 1;
302-
case ZEND_ASSIGN_DIM_OP:
303327
case ZEND_ASSIGN_OBJ_OP:
304328
case ZEND_ASSIGN_STATIC_PROP_OP:
305-
case ZEND_ASSIGN_DIM:
306329
case ZEND_ASSIGN_OBJ:
307330
break;
308331
case ZEND_ASSIGN_OBJ_REF:
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
--TEST--
2+
Escape analysis: objects implementing ArrayAccess escape through dimension accesses
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=-1
7+
--EXTENSIONS--
8+
opcache
9+
--FILE--
10+
<?php
11+
#[AllowDynamicProperties]
12+
class AA implements ArrayAccess {
13+
function offsetExists($o): bool { $this->x = 3; return true; }
14+
function offsetGet($o): mixed { $this->x = 2; return 42; }
15+
function offsetSet($o, $v): void { $this->x = 4; }
16+
function offsetUnset($o): void {}
17+
18+
static function fetch() {
19+
$o = new self;
20+
$o->x = 1;
21+
$o[0];
22+
return $o->x;
23+
}
24+
25+
static function isset() {
26+
$o = new self;
27+
$o->x = 1;
28+
return [isset($o[0]), $o->x];
29+
}
30+
31+
static function assign() {
32+
$o = new self;
33+
$o->x = 1;
34+
$o[0] = 1;
35+
return $o->x;
36+
}
37+
}
38+
39+
#[AllowDynamicProperties] class B {}
40+
#[AllowDynamicProperties]
41+
class BB implements ArrayAccess {
42+
function offsetExists($o): bool {
43+
return true;
44+
}
45+
46+
function offsetGet($o): mixed {
47+
$this->x = 2;
48+
return 1;
49+
}
50+
51+
function offsetSet($o, $v): void {}
52+
53+
function offsetUnset($o): void {}
54+
55+
static function f($c) {
56+
if ($c) { $o = new B; $o->x = 1; } else { $o = new self; $o->x = 1; }
57+
$o[0];
58+
return $o->x;
59+
}
60+
}
61+
62+
var_dump(AA::fetch());
63+
var_dump(AA::isset());
64+
var_dump(AA::assign());
65+
var_dump(BB::f(false));
66+
?>
67+
--EXPECT--
68+
int(2)
69+
array(2) {
70+
[0]=>
71+
bool(true)
72+
[1]=>
73+
int(3)
74+
}
75+
int(4)
76+
int(2)

0 commit comments

Comments
 (0)