Skip to content

Commit d556961

Browse files
committed
Fix accessibility checks for dynamic properties
A dynamic property may be shadowed by a private/protected property. Make sure we check property accessibility for non-indirect properties as well.
1 parent 251bd9c commit d556961

7 files changed

+80
-31
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Dynamic property shadowed by private property
3+
--FILE--
4+
<?php
5+
6+
class Test {
7+
private $prop = "Test";
8+
9+
function run() {
10+
foreach ($this as $k => $v) {
11+
echo "$k => $v\n";
12+
}
13+
var_dump(get_object_vars($this));
14+
}
15+
}
16+
class Test2 extends Test {
17+
}
18+
19+
$test2 = new Test2;
20+
$test2->prop = "Test2";
21+
$test2->run();
22+
23+
?>
24+
--EXPECT--
25+
prop => Test
26+
array(1) {
27+
["prop"]=>
28+
string(4) "Test"
29+
}

Zend/zend_builtin_functions.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,18 +1191,18 @@ ZEND_FUNCTION(get_object_vars)
11911191
array_init_size(return_value, zend_hash_num_elements(properties));
11921192

11931193
ZEND_HASH_FOREACH_KEY_VAL(properties, num_key, key, value) {
1194-
zend_bool unmangle = 0;
1194+
zend_bool is_dynamic = 1;
11951195
if (Z_TYPE_P(value) == IS_INDIRECT) {
11961196
value = Z_INDIRECT_P(value);
11971197
if (UNEXPECTED(Z_ISUNDEF_P(value))) {
11981198
continue;
11991199
}
12001200

1201-
ZEND_ASSERT(key);
1202-
if (zend_check_property_access(zobj, key) == FAILURE) {
1203-
continue;
1204-
}
1205-
unmangle = 1;
1201+
is_dynamic = 0;
1202+
}
1203+
1204+
if (key && zend_check_property_access(zobj, key, is_dynamic) == FAILURE) {
1205+
continue;
12061206
}
12071207

12081208
if (Z_ISREF_P(value) && Z_REFCOUNT_P(value) == 1) {
@@ -1213,7 +1213,7 @@ ZEND_FUNCTION(get_object_vars)
12131213
if (UNEXPECTED(!key)) {
12141214
/* This case is only possible due to loopholes, e.g. ArrayObject */
12151215
zend_hash_index_add(Z_ARRVAL_P(return_value), num_key, value);
1216-
} else if (unmangle && ZSTR_VAL(key)[0] == 0) {
1216+
} else if (!is_dynamic && ZSTR_VAL(key)[0] == 0) {
12171217
const char *prop_name, *class_name;
12181218
size_t prop_len;
12191219
zend_unmangle_property_name_ex(key, &class_name, &prop_name, &prop_len);

Zend/zend_object_handlers.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ ZEND_API zend_property_info *zend_get_property_info(zend_class_entry *ce, zend_s
525525
}
526526
/* }}} */
527527

528-
ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name) /* {{{ */
528+
ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name, zend_bool is_dynamic) /* {{{ */
529529
{
530530
zend_property_info *property_info;
531531
const char *class_name = NULL;
@@ -534,19 +534,18 @@ ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_inf
534534
size_t prop_name_len;
535535

536536
if (ZSTR_VAL(prop_info_name)[0] == 0) {
537+
if (is_dynamic) {
538+
return SUCCESS;
539+
}
540+
537541
zend_unmangle_property_name_ex(prop_info_name, &class_name, &prop_name, &prop_name_len);
538542
member = zend_string_init(prop_name, prop_name_len, 0);
539543
property_info = zend_get_property_info(zobj->ce, member, 1);
540544
zend_string_release_ex(member, 0);
541-
if (property_info == NULL) {
542-
if (class_name[0] != '*') {
543-
/* we we're looking for a private prop */
544-
return FAILURE;
545-
}
546-
return SUCCESS;
547-
} else if (property_info == ZEND_WRONG_PROPERTY_INFO) {
545+
if (property_info == NULL || property_info == ZEND_WRONG_PROPERTY_INFO) {
548546
return FAILURE;
549547
}
548+
550549
if (class_name[0] != '*') {
551550
if (!(property_info->flags & ZEND_ACC_PRIVATE)) {
552551
/* we we're looking for a private prop but found a non private one of the same name */
@@ -562,6 +561,7 @@ ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_inf
562561
} else {
563562
property_info = zend_get_property_info(zobj->ce, prop_info_name, 1);
564563
if (property_info == NULL) {
564+
ZEND_ASSERT(is_dynamic);
565565
return SUCCESS;
566566
} else if (property_info == ZEND_WRONG_PROPERTY_INFO) {
567567
return FAILURE;

Zend/zend_object_handlers.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ ZEND_API void rebuild_object_properties(zend_object *zobj);
224224

225225
ZEND_API int zend_check_protected(zend_class_entry *ce, zend_class_entry *scope);
226226

227-
ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name);
227+
ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name, zend_bool is_dynamic);
228228

229229
ZEND_API zend_function *zend_get_call_trampoline_func(zend_class_entry *ce, zend_string *method_name, int is_static);
230230

Zend/zend_vm_def.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5845,10 +5845,12 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
58455845
value = Z_INDIRECT_P(value);
58465846
value_type = Z_TYPE_INFO_P(value);
58475847
if (EXPECTED(value_type != IS_UNDEF)
5848-
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
5848+
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
58495849
break;
58505850
}
5851-
} else {
5851+
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
5852+
|| !p->key
5853+
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
58525854
break;
58535855
}
58545856
}
@@ -5995,10 +5997,12 @@ ZEND_VM_HANDLER(126, ZEND_FE_FETCH_RW, VAR, ANY, JMP_ADDR)
59955997
value = Z_INDIRECT_P(value);
59965998
value_type = Z_TYPE_INFO_P(value);
59975999
if (EXPECTED(value_type != IS_UNDEF)
5998-
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
6000+
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
59996001
break;
60006002
}
6001-
} else {
6003+
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
6004+
|| !p->key
6005+
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
60026006
break;
60036007
}
60046008
}

Zend/zend_vm_execute.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21258,10 +21258,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_R_SPEC_VAR_HANDLER(ZE
2125821258
value = Z_INDIRECT_P(value);
2125921259
value_type = Z_TYPE_INFO_P(value);
2126021260
if (EXPECTED(value_type != IS_UNDEF)
21261-
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
21261+
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
2126221262
break;
2126321263
}
21264-
} else {
21264+
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
21265+
|| !p->key
21266+
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
2126521267
break;
2126621268
}
2126721269
}
@@ -21408,10 +21410,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_RW_SPEC_VAR_HANDLER(Z
2140821410
value = Z_INDIRECT_P(value);
2140921411
value_type = Z_TYPE_INFO_P(value);
2141021412
if (EXPECTED(value_type != IS_UNDEF)
21411-
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
21413+
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
2141221414
break;
2141321415
}
21414-
} else {
21416+
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
21417+
|| !p->key
21418+
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
2141521419
break;
2141621420
}
2141721421
}

ext/standard/http.c

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,29 @@ PHPAPI int php_url_encode_hash_ex(HashTable *ht, smart_str *formstr,
5353
}
5454
arg_sep_len = strlen(arg_sep);
5555

56-
ZEND_HASH_FOREACH_KEY_VAL_IND(ht, idx, key, zdata) {
56+
ZEND_HASH_FOREACH_KEY_VAL(ht, idx, key, zdata) {
57+
zend_bool is_dynamic = 1;
58+
if (Z_TYPE_P(zdata) == IS_INDIRECT) {
59+
zdata = Z_INDIRECT_P(zdata);
60+
if (Z_ISUNDEF_P(zdata)) {
61+
continue;
62+
}
63+
64+
is_dynamic = 0;
65+
}
66+
5767
/* handling for private & protected object properties */
5868
if (key) {
69+
prop_name = ZSTR_VAL(key);
70+
prop_len = ZSTR_LEN(key);
71+
72+
if (type != NULL && zend_check_property_access(Z_OBJ_P(type), key, is_dynamic) != SUCCESS) {
73+
/* property not visible in this scope */
74+
continue;
75+
}
76+
5977
if (ZSTR_VAL(key)[0] == '\0' && type != NULL) {
6078
const char *tmp;
61-
62-
zend_object *zobj = Z_OBJ_P(type);
63-
if (zend_check_property_access(zobj, key) != SUCCESS) {
64-
/* private or protected property access outside of the class */
65-
continue;
66-
}
6779
zend_unmangle_property_name_ex(key, &tmp, &prop_name, &prop_len);
6880
} else {
6981
prop_name = ZSTR_VAL(key);

0 commit comments

Comments
 (0)