Skip to content

Commit 0efecbc

Browse files
committed
Fix by-ref assignment to uninitialized hooked backing value
Within hooks, the backing value can directly be accessed as if no hooks were present. This was previously handled only in read_property(). zend_fetch_property_address(), which is used for by-ref assignment, will first call get_property_ptr_ptr() and then try read_property(). However, when called on uninitialized backing values, read_property() will return &EG(uninitialized_zval) with an uninitialized property warning. This is problematic for zend_fetch_property_address() because it write to the result of read_property() unless there's an exception. For untyped properties, this can result in writes to &EG(uninitialized_zval) (see oss-fuzz-471486164-001.phpt). For types properties, it will result in an unexpected "Typed property C::$prop must not be accessed before initialization" exception. Fixes OSS-Fuzz #471486164 Closes phpGH-20943
1 parent 462fcad commit 0efecbc

File tree

4 files changed

+60
-1
lines changed

4 files changed

+60
-1
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ PHP NEWS
1010
. Fixed bug GH-GH-20914 (Internal enums can be cloned and compared). (Arnaud)
1111
. Fix OSS-Fuzz #474613951 (Leaked parent property default value). (ilutov)
1212
. Fixed bug GH-20766 (Use-after-free in FE_FREE with GC interaction). (Bob)
13+
. Fix OSS-Fuzz #471486164 (Broken by-ref assignment to uninitialized hooked
14+
backing value). (ilutov)
1315

1416
- Date:
1517
. Update timelib to 2022.16. (Derick)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
OSS-Fuzz #471486164: get_property_ptr_ptr() on uninitialized hooked property
3+
--FILE--
4+
<?php
5+
6+
class C {
7+
public $a {
8+
get => $this->a;
9+
set { $this->a = &$value; }
10+
}
11+
public $x = 1;
12+
}
13+
14+
$proxy = (new ReflectionClass(C::class))->newLazyProxy(function ($proxy) {
15+
$proxy->a = 1;
16+
return new C;
17+
});
18+
var_dump($proxy->x);
19+
20+
?>
21+
--EXPECT--
22+
int(1)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
OSS-Fuzz #471486164: get_property_ptr_ptr() on uninitialized hooked property
3+
--FILE--
4+
<?php
5+
6+
class C {
7+
public int $a {
8+
get => $this->a;
9+
set {
10+
global $ref;
11+
$this->a = &$ref;
12+
}
13+
}
14+
}
15+
16+
$ref = 1;
17+
$proxy = new C;
18+
$proxy->a = 1;
19+
var_dump($proxy->a);
20+
$ref++;
21+
var_dump($proxy->a);
22+
23+
?>
24+
--EXPECT--
25+
int(1)
26+
int(2)

Zend/zend_object_handlers.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
13921392
property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__get != NULL), cache_slot, &prop_info);
13931393

13941394
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
1395+
try_again:
13951396
retval = OBJ_PROP(zobj, property_offset);
13961397
if (UNEXPECTED(Z_TYPE_P(retval) == IS_UNDEF)) {
13971398
if (EXPECTED(!zobj->ce->__get) ||
@@ -1471,7 +1472,15 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14711472
}
14721473
retval = zend_hash_add(zobj->properties, name, &EG(uninitialized_zval));
14731474
}
1474-
} else if (!IS_HOOKED_PROPERTY_OFFSET(property_offset) && zobj->ce->__get == NULL) {
1475+
} else if (IS_HOOKED_PROPERTY_OFFSET(property_offset)) {
1476+
if (!(prop_info->flags & ZEND_ACC_VIRTUAL) && !zend_should_call_hook(prop_info, zobj)) {
1477+
property_offset = prop_info->offset;
1478+
if (!ZEND_TYPE_IS_SET(prop_info->type)) {
1479+
prop_info = NULL;
1480+
}
1481+
goto try_again;
1482+
}
1483+
} else if (zobj->ce->__get == NULL) {
14751484
retval = &EG(error_zval);
14761485
}
14771486

0 commit comments

Comments
 (0)