Skip to content

Commit

Permalink
fix bug #61782 - __clone/__destruct do not match other methods when c…
Browse files Browse the repository at this point in the history
…hecking access controls
  • Loading branch information
smalyshev committed May 14, 2012
1 parent 47db8a9 commit d03900d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 38 deletions.
29 changes: 29 additions & 0 deletions Zend/tests/bug61782.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Bug #61782 (__clone/__destruct do not match other methods when checking access controls)
--FILE--
<?php
abstract class BaseClass {
abstract protected function __clone();
}

class MommasBoy extends BaseClass {
protected function __clone() {
echo __METHOD__, "\n";
}
}

class LatchkeyKid extends BaseClass {
public function __construct() {
echo 'In ', __CLASS__, ":\n";
$kid = new MommasBoy();
$kid = clone $kid;
}
public function __clone() {}
}

$obj = new LatchkeyKid();
echo "DONE\n";
--EXPECT--
In LatchkeyKid:
MommasBoy::__clone
DONE
28 changes: 11 additions & 17 deletions Zend/zend_object_handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,24 @@ ZEND_API void rebuild_object_properties(zend_object *zobj) /* {{{ */
zend_hash_get_current_data_ex(&ce->properties_info, (void**)&prop_info, &pos) == SUCCESS;
zend_hash_move_forward_ex(&ce->properties_info, &pos)) {
if (/*prop_info->ce == ce &&*/
(prop_info->flags & ZEND_ACC_STATIC) == 0 &&
(prop_info->flags & ZEND_ACC_STATIC) == 0 &&
prop_info->offset >= 0 &&
zobj->properties_table[prop_info->offset]) {
zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]);
}
}
}
while (ce->parent && ce->parent->default_properties_count) {
ce = ce->parent;
for (zend_hash_internal_pointer_reset_ex(&ce->properties_info, &pos);
zend_hash_get_current_data_ex(&ce->properties_info, (void**)&prop_info, &pos) == SUCCESS;
zend_hash_move_forward_ex(&ce->properties_info, &pos)) {
if (prop_info->ce == ce &&
(prop_info->flags & ZEND_ACC_STATIC) == 0 &&
(prop_info->flags & ZEND_ACC_PRIVATE) != 0 &&
(prop_info->flags & ZEND_ACC_STATIC) == 0 &&
(prop_info->flags & ZEND_ACC_PRIVATE) != 0 &&
prop_info->offset >= 0 &&
zobj->properties_table[prop_info->offset]) {
zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]);
}
}
}
}
}
Expand Down Expand Up @@ -783,7 +783,7 @@ static void zend_std_unset_property(zval *object, zval *member, const zend_liter
property_info = zend_get_property_info_quick(zobj->ce, member, (zobj->ce->__unset != NULL), key TSRMLS_CC);

if (EXPECTED(property_info != NULL) &&
EXPECTED((property_info->flags & ZEND_ACC_STATIC) == 0) &&
EXPECTED((property_info->flags & ZEND_ACC_STATIC) == 0) &&
!zobj->properties &&
property_info->offset >= 0 &&
EXPECTED(zobj->properties_table[property_info->offset] != NULL)) {
Expand Down Expand Up @@ -815,8 +815,8 @@ static void zend_std_unset_property(zval *object, zval *member, const zend_liter
}
}
}
} else if (EXPECTED(property_info != NULL) &&
EXPECTED((property_info->flags & ZEND_ACC_STATIC) == 0) &&
} else if (EXPECTED(property_info != NULL) &&
EXPECTED((property_info->flags & ZEND_ACC_STATIC) == 0) &&
property_info->offset >= 0) {
zobj->properties_table[property_info->offset] = NULL;
}
Expand Down Expand Up @@ -960,12 +960,6 @@ ZEND_API int zend_check_protected(zend_class_entry *ce, zend_class_entry *scope)
}
/* }}} */

static inline zend_class_entry * zend_get_function_root_class(zend_function *fbc) /* {{{ */
{
return fbc->common.prototype ? fbc->common.prototype->common.scope : fbc->common.scope;
}
/* }}} */

static inline union _zend_function *zend_get_user_call_function(zend_class_entry *ce, const char *method_name, int method_len) /* {{{ */
{
zend_internal_function *call_user_call = emalloc(sizeof(zend_internal_function));
Expand Down Expand Up @@ -1143,7 +1137,7 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, const c
zend_str_tolower_copy(lc_function_name, function_name_strval, function_name_strlen);
hash_value = zend_hash_func(lc_function_name, function_name_strlen+1);
}

if (function_name_strlen == ce->name_length && ce->constructor) {
lc_class_name = zend_str_tolower_dup(ce->name, ce->name_length);
/* Only change the method to the constructor if the constructor isn't called __construct
Expand Down Expand Up @@ -1178,7 +1172,7 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, const c
if (UNEXPECTED(!(fbc->common.fn_flags & ZEND_ACC_STATIC))) {
zend_error_noreturn(E_ERROR, "Cannot call non static method %s::%s() without object", ZEND_FN_SCOPE_NAME(fbc), fbc->common.function_name);
}
#endif
#endif
if (fbc->op_array.fn_flags & ZEND_ACC_PUBLIC) {
/* No further checks necessary, most common case */
} else if (fbc->op_array.fn_flags & ZEND_ACC_PRIVATE) {
Expand Down Expand Up @@ -1220,7 +1214,7 @@ ZEND_API zval **zend_std_get_static_property(zend_class_entry *ce, const char *p
{
zend_property_info *property_info;
ulong hash_value;

if (UNEXPECTED(!key) ||
(property_info = CACHED_POLYMORPHIC_PTR(key->cache_slot, ce)) == NULL) {
if (EXPECTED(key != NULL)) {
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_object_handlers.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ struct _zend_object_handlers {

extern ZEND_API zend_object_handlers std_object_handlers;

#define zend_get_function_root_class(fbc) \
((fbc)->common.prototype ? (fbc)->common.prototype->common.scope : (fbc)->common.scope)

BEGIN_EXTERN_C()
ZEND_API union _zend_function *zend_std_get_static_method(zend_class_entry *ce, const char *function_name_strval, int function_name_strlen, const struct _zend_literal *key TSRMLS_DC);
ZEND_API zval **zend_std_get_static_property(zend_class_entry *ce, const char *property_name, int property_name_len, zend_bool silent, const struct _zend_literal *key TSRMLS_DC);
Expand Down
26 changes: 13 additions & 13 deletions Zend/zend_objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

ZEND_API void zend_object_std_init(zend_object *object, zend_class_entry *ce TSRMLS_DC)
{
object->ce = ce;
object->ce = ce;
object->properties = NULL;
object->properties_table = NULL;
object->guards = NULL;
Expand All @@ -38,7 +38,7 @@ ZEND_API void zend_object_std_dtor(zend_object *object TSRMLS_DC)
{
if (object->guards) {
zend_hash_destroy(object->guards);
FREE_HASHTABLE(object->guards);
FREE_HASHTABLE(object->guards);
}
if (object->properties) {
zend_hash_destroy(object->properties);
Expand Down Expand Up @@ -74,23 +74,23 @@ ZEND_API void zend_objects_destroy_object(zend_object *object, zend_object_handl
if (object->ce != EG(scope)) {
zend_class_entry *ce = object->ce;

zend_error(EG(in_execution) ? E_ERROR : E_WARNING,
"Call to private %s::__destruct() from context '%s'%s",
ce->name,
EG(scope) ? EG(scope)->name : "",
zend_error(EG(in_execution) ? E_ERROR : E_WARNING,
"Call to private %s::__destruct() from context '%s'%s",
ce->name,
EG(scope) ? EG(scope)->name : "",
EG(in_execution) ? "" : " during shutdown ignored");
return;
}
} else {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (!zend_check_protected(destructor->common.scope, EG(scope))) {
if (!zend_check_protected(zend_get_function_root_class(destructor), EG(scope))) {
zend_class_entry *ce = object->ce;

zend_error(EG(in_execution) ? E_ERROR : E_WARNING,
"Call to protected %s::__destruct() from context '%s'%s",
ce->name,
EG(scope) ? EG(scope)->name : "",
zend_error(EG(in_execution) ? E_ERROR : E_WARNING,
"Call to protected %s::__destruct() from context '%s'%s",
ce->name,
EG(scope) ? EG(scope)->name : "",
EG(in_execution) ? "" : " during shutdown ignored");
return;
}
Expand Down Expand Up @@ -139,7 +139,7 @@ ZEND_API void zend_objects_free_object_storage(zend_object *object TSRMLS_DC)
}

ZEND_API zend_object_value zend_objects_new(zend_object **object, zend_class_entry *class_type TSRMLS_DC)
{
{
zend_object_value retval;

*object = emalloc(sizeof(zend_object));
Expand Down Expand Up @@ -222,7 +222,7 @@ ZEND_API zend_object_value zend_objects_clone_obj(zval *zobject TSRMLS_DC)
zend_object *new_object;
zend_object_handle handle = Z_OBJ_HANDLE_P(zobject);

/* assume that create isn't overwritten, so when clone depends on the
/* assume that create isn't overwritten, so when clone depends on the
* overwritten one then it must itself be overwritten */
old_object = zend_objects_get_address(zobject TSRMLS_CC);
new_obj_val = zend_objects_new(&new_object, old_object->ce TSRMLS_CC);
Expand Down
6 changes: 3 additions & 3 deletions Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -2451,7 +2451,7 @@ ZEND_VM_HANDLER(59, ZEND_INIT_FCALL_BY_NAME, ANY, CONST|TMP|VAR|CV)
if (UNEXPECTED(EX(fbc) == NULL)) {
zend_error_noreturn(E_ERROR, "Call to undefined method %s::%s()", Z_OBJ_CLASS_NAME_P(EX(object)), Z_STRVAL_PP(method));
}

if ((EX(fbc)->common.fn_flags & ZEND_ACC_STATIC) != 0) {
EX(object) = NULL;
} else {
Expand Down Expand Up @@ -2983,7 +2983,7 @@ ZEND_VM_HANDLER(107, ZEND_CATCH, CONST, CV)
catch_ce = CACHED_PTR(opline->op1.literal->cache_slot);
} else {
catch_ce = zend_fetch_class_by_name(Z_STRVAL_P(opline->op1.zv), Z_STRLEN_P(opline->op1.zv), opline->op1.literal + 1, ZEND_FETCH_CLASS_NO_AUTOLOAD TSRMLS_CC);

CACHE_PTR(opline->op1.literal->cache_slot, catch_ce);
}
ce = Z_OBJCE_P(EG(exception));
Expand Down Expand Up @@ -3435,7 +3435,7 @@ ZEND_VM_HANDLER(110, ZEND_CLONE, CONST|TMP|VAR|UNUSED|CV, ANY)
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down
10 changes: 5 additions & 5 deletions Zend/zend_vm_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -2436,7 +2436,7 @@ static int ZEND_FASTCALL ZEND_CLONE_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down Expand Up @@ -6970,7 +6970,7 @@ static int ZEND_FASTCALL ZEND_CLONE_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down Expand Up @@ -11518,7 +11518,7 @@ static int ZEND_FASTCALL ZEND_CLONE_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down Expand Up @@ -21751,7 +21751,7 @@ static int ZEND_FASTCALL ZEND_CLONE_SPEC_UNUSED_HANDLER(ZEND_OPCODE_HANDLER_ARG
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down Expand Up @@ -27498,7 +27498,7 @@ static int ZEND_FASTCALL ZEND_CLONE_SPEC_CV_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
} else if ((clone->common.fn_flags & ZEND_ACC_PROTECTED)) {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (UNEXPECTED(!zend_check_protected(clone->common.scope, EG(scope)))) {
if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), EG(scope)))) {
zend_error_noreturn(E_ERROR, "Call to protected %s::__clone() from context '%s'", ce->name, EG(scope) ? EG(scope)->name : "");
}
}
Expand Down

0 comments on commit d03900d

Please sign in to comment.