Skip to content

Commit 01ca5f7

Browse files
committed
Disallow access of backing value outside of hook
1 parent 6029234 commit 01ca5f7

File tree

3 files changed

+106
-0
lines changed

3 files changed

+106
-0
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
--TEST--
2+
Attempted read/write of backing value in a delegated method throws
3+
--FILE--
4+
<?php
5+
6+
class Test {
7+
public $prop = 42 {
8+
get => $this->getProp($this->prop);
9+
set {
10+
$this->setProp($this->prop, $value);
11+
}
12+
}
13+
14+
private function getProp($prop) {
15+
return $this->prop;
16+
}
17+
18+
private function setProp($prop, $value) {
19+
$this->prop = $value;
20+
}
21+
22+
public $prop2 = 42 {
23+
get => $this->prop2;
24+
set => $value;
25+
}
26+
}
27+
28+
class Child extends Test {
29+
public $prop2 = 42 {
30+
get => parent::$prop2::get();
31+
set { parent::$prop2::set($value); }
32+
}
33+
}
34+
35+
$test = new Test;
36+
37+
try {
38+
$test->prop = 0;
39+
} catch (Error $e) {
40+
echo $e->getMessage(), "\n";
41+
}
42+
43+
try {
44+
var_dump($test->prop);
45+
} catch (Error $e) {
46+
echo $e->getMessage(), "\n";
47+
}
48+
49+
$child = new Child();
50+
$child->prop2 = 43;
51+
var_dump($child->prop2);
52+
53+
?>
54+
--EXPECT--
55+
Must not write to property Test::$prop recursively
56+
Must not read from property Test::$prop recursively
57+
int(43)

Zend/zend_inheritance.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,8 @@ static void inherit_property_hook(
13151315
return;
13161316
}
13171317

1318+
child->common.prototype = parent->common.prototype ? parent->common.prototype : parent;
1319+
13181320
uint32_t parent_flags = parent->common.fn_flags;
13191321
if (parent_flags & ZEND_ACC_PRIVATE) {
13201322
child->common.fn_flags |= ZEND_ACC_CHANGED;

Zend/zend_object_handlers.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,44 @@ ZEND_COLD static void zend_typed_property_uninitialized_access(const zend_proper
623623
ZSTR_VAL(name));
624624
}
625625

626+
static ZEND_FUNCTION(zend_parent_hook_get_trampoline);
627+
static ZEND_FUNCTION(zend_parent_hook_set_trampoline);
628+
629+
static bool is_in_hook(const zend_property_info *prop_info)
630+
{
631+
zend_execute_data *execute_data = EG(current_execute_data);
632+
if (!execute_data || !EX(func)) {
633+
return false;
634+
}
635+
636+
zend_function *func = EX(func);
637+
zend_function **hooks = prop_info->hooks;
638+
ZEND_ASSERT(hooks);
639+
640+
/* Fast path: Check if we're directly in the properties hook. */
641+
if (func == hooks[ZEND_PROPERTY_HOOK_GET]
642+
|| func == hooks[ZEND_PROPERTY_HOOK_SET]) {
643+
return true;
644+
}
645+
646+
/* Check if we're in a parent hook. */
647+
zend_function *prototype = func->common.prototype ? func->common.prototype : func;
648+
if ((hooks[ZEND_PROPERTY_HOOK_GET] && prototype == hooks[ZEND_PROPERTY_HOOK_GET]->common.prototype)
649+
|| (hooks[ZEND_PROPERTY_HOOK_SET] && prototype == hooks[ZEND_PROPERTY_HOOK_SET]->common.prototype)) {
650+
return true;
651+
}
652+
653+
/* Check if we're in the trampoline helper. */
654+
if (EX(func) == &EG(trampoline)
655+
&& !ZEND_USER_CODE(EX(func)->type)
656+
&& (EX(func)->internal_function.handler == ZEND_FN(zend_parent_hook_get_trampoline)
657+
|| EX(func)->internal_function.handler == ZEND_FN(zend_parent_hook_set_trampoline))) {
658+
return true;
659+
}
660+
661+
return false;
662+
}
663+
626664
ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int type, void **cache_slot, zval *rv) /* {{{ */
627665
{
628666
zval *retval;
@@ -738,6 +776,9 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
738776
zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
739777
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
740778
return &EG(uninitialized_zval);
779+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
780+
zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
781+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
741782
}
742783
property_offset = prop_info->offset;
743784
if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -1000,6 +1041,9 @@ found:;
10001041
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
10011042
variable_ptr = &EG(error_zval);
10021043
goto exit;
1044+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
1045+
zend_throw_error(NULL, "Must not write to property %s::$%s recursively",
1046+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
10031047
}
10041048
property_offset = prop_info->offset;
10051049
if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -2115,6 +2159,9 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
21152159
zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
21162160
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
21172161
return 0;
2162+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
2163+
zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
2164+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
21182165
}
21192166
property_offset = prop_info->offset;
21202167
if (!ZEND_TYPE_IS_SET(prop_info->type)) {

0 commit comments

Comments
 (0)