Skip to content

Fix accessibility checks for dynamic properties #3626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix accessibility checks for dynamic properties #3626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Zend/tests/foreach_shadowed_dyn_property.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Dynamic property shadowed by private property
--FILE--
<?php

class Test {
private $prop = "Test";

function run() {
foreach ($this as $k => $v) {
echo "$k => $v\n";
}
var_dump(get_object_vars($this));
}
}
class Test2 extends Test {
}

$test2 = new Test2;
$test2->prop = "Test2";
$test2->run();

?>
--EXPECT--
prop => Test
array(1) {
["prop"]=>
string(4) "Test"
}
14 changes: 7 additions & 7 deletions Zend/zend_builtin_functions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,18 +1191,18 @@ ZEND_FUNCTION(get_object_vars)
array_init_size(return_value, zend_hash_num_elements(properties));

ZEND_HASH_FOREACH_KEY_VAL(properties, num_key, key, value) {
zend_bool unmangle = 0;
zend_bool is_dynamic = 1;
if (Z_TYPE_P(value) == IS_INDIRECT) {
value = Z_INDIRECT_P(value);
if (UNEXPECTED(Z_ISUNDEF_P(value))) {
continue;
}

ZEND_ASSERT(key);
if (zend_check_property_access(zobj, key) == FAILURE) {
continue;
}
unmangle = 1;
is_dynamic = 0;
}

if (key && zend_check_property_access(zobj, key, is_dynamic) == FAILURE) {
continue;
}

if (Z_ISREF_P(value) && Z_REFCOUNT_P(value) == 1) {
Expand All @@ -1213,7 +1213,7 @@ ZEND_FUNCTION(get_object_vars)
if (UNEXPECTED(!key)) {
/* This case is only possible due to loopholes, e.g. ArrayObject */
zend_hash_index_add(Z_ARRVAL_P(return_value), num_key, value);
} else if (unmangle && ZSTR_VAL(key)[0] == 0) {
} else if (!is_dynamic && ZSTR_VAL(key)[0] == 0) {
const char *prop_name, *class_name;
size_t prop_len;
zend_unmangle_property_name_ex(key, &class_name, &prop_name, &prop_len);
Expand Down
16 changes: 8 additions & 8 deletions Zend/zend_object_handlers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ ZEND_API zend_property_info *zend_get_property_info(zend_class_entry *ce, zend_s
}
/* }}} */

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

if (ZSTR_VAL(prop_info_name)[0] == 0) {
if (is_dynamic) {
return SUCCESS;
}

zend_unmangle_property_name_ex(prop_info_name, &class_name, &prop_name, &prop_name_len);
member = zend_string_init(prop_name, prop_name_len, 0);
property_info = zend_get_property_info(zobj->ce, member, 1);
zend_string_release_ex(member, 0);
if (property_info == NULL) {
if (class_name[0] != '*') {
/* we we're looking for a private prop */
return FAILURE;
}
return SUCCESS;
} else if (property_info == ZEND_WRONG_PROPERTY_INFO) {
if (property_info == NULL || property_info == ZEND_WRONG_PROPERTY_INFO) {
return FAILURE;
}

if (class_name[0] != '*') {
if (!(property_info->flags & ZEND_ACC_PRIVATE)) {
/* we we're looking for a private prop but found a non private one of the same name */
Expand All @@ -562,6 +561,7 @@ ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_inf
} else {
property_info = zend_get_property_info(zobj->ce, prop_info_name, 1);
if (property_info == NULL) {
ZEND_ASSERT(is_dynamic);
return SUCCESS;
} else if (property_info == ZEND_WRONG_PROPERTY_INFO) {
return FAILURE;
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_object_handlers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ ZEND_API void rebuild_object_properties(zend_object *zobj);

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

ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name);
ZEND_API int zend_check_property_access(zend_object *zobj, zend_string *prop_info_name, zend_bool is_dynamic);

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

Expand Down
12 changes: 8 additions & 4 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5845,10 +5845,12 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
} else {
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
|| !p->key
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
break;
}
}
Expand Down Expand Up @@ -5995,10 +5997,12 @@ ZEND_VM_HANDLER(126, ZEND_FE_FETCH_RW, VAR, ANY, JMP_ADDR)
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
} else {
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
|| !p->key
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
break;
}
}
Expand Down
12 changes: 8 additions & 4 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21258,10 +21258,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_R_SPEC_VAR_HANDLER(ZE
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
} else {
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
|| !p->key
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
break;
}
}
Expand Down Expand Up @@ -21408,10 +21410,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_RW_SPEC_VAR_HANDLER(Z
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key) == SUCCESS)) {
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
} else {
} else if (EXPECTED(Z_OBJCE_P(array)->default_properties_count == 0)
|| !p->key
|| zend_check_property_access(Z_OBJ_P(array), p->key, 1) == SUCCESS) {
break;
}
}
Expand Down
26 changes: 19 additions & 7 deletions ext/standard/http.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,29 @@ PHPAPI int php_url_encode_hash_ex(HashTable *ht, smart_str *formstr,
}
arg_sep_len = strlen(arg_sep);

ZEND_HASH_FOREACH_KEY_VAL_IND(ht, idx, key, zdata) {
ZEND_HASH_FOREACH_KEY_VAL(ht, idx, key, zdata) {
zend_bool is_dynamic = 1;
if (Z_TYPE_P(zdata) == IS_INDIRECT) {
zdata = Z_INDIRECT_P(zdata);
if (Z_ISUNDEF_P(zdata)) {
continue;
}

is_dynamic = 0;
}

/* handling for private & protected object properties */
if (key) {
prop_name = ZSTR_VAL(key);
prop_len = ZSTR_LEN(key);

if (type != NULL && zend_check_property_access(Z_OBJ_P(type), key, is_dynamic) != SUCCESS) {
/* property not visible in this scope */
continue;
}

if (ZSTR_VAL(key)[0] == '\0' && type != NULL) {
const char *tmp;

zend_object *zobj = Z_OBJ_P(type);
if (zend_check_property_access(zobj, key) != SUCCESS) {
/* private or protected property access outside of the class */
continue;
}
zend_unmangle_property_name_ex(key, &tmp, &prop_name, &prop_len);
} else {
prop_name = ZSTR_VAL(key);
Expand Down