Skip to content

Commit 699ff29

Browse files
committed
Disallow access of backing value outside of hook
1 parent 14066a4 commit 699ff29

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;
@@ -739,6 +777,9 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
739777
zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
740778
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
741779
return &EG(uninitialized_zval);
780+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
781+
zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
782+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
742783
}
743784
property_offset = prop_info->offset;
744785
if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -1006,6 +1047,9 @@ found:;
10061047
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
10071048
variable_ptr = &EG(error_zval);
10081049
goto exit;
1050+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
1051+
zend_throw_error(NULL, "Must not write to property %s::$%s recursively",
1052+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
10091053
}
10101054
property_offset = prop_info->offset;
10111055
if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -2122,6 +2166,9 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
21222166
zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
21232167
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
21242168
return 0;
2169+
} else if (UNEXPECTED(!is_in_hook(prop_info))) {
2170+
zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
2171+
ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
21252172
}
21262173
property_offset = prop_info->offset;
21272174
if (!ZEND_TYPE_IS_SET(prop_info->type)) {

0 commit comments

Comments
 (0)