Zend: Add zend_check_method_accessible() to DRY method visibility checks (#18995)

* Zend: Add `zend_check_method_accessible()` to DRY method visibility checks

* Zend: Add assertions verifying flags didn't change before `zend_check_method_accessible()`

* Try `zend_always_inline` for `zend_check_method_accessible`
This commit is contained in:
Tim Düsterhus 2025-07-07 21:30:13 +02:00 committed by GitHub
parent 9225cb45ac
commit 45d948f2da
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 90 additions and 139 deletions

View file

@ -4021,15 +4021,13 @@ static zend_always_inline bool zend_is_callable_check_func(zval *callable, zend_
((fcc->object && fcc->calling_scope->__call) ||
(!fcc->object && fcc->calling_scope->__callstatic)))) {
scope = get_scope(frame);
if (fcc->function_handler->common.scope != scope) {
if ((fcc->function_handler->common.fn_flags & ZEND_ACC_PRIVATE)
|| !zend_check_protected(zend_get_function_root_class(fcc->function_handler), scope)) {
ZEND_ASSERT(!(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(fcc->function_handler, scope)) {
retval = 0;
fcc->function_handler = NULL;
goto get_function_via_handler;
}
}
}
} else {
get_function_via_handler:
if (fcc->object && fcc->calling_scope == ce_org) {
@ -4086,9 +4084,8 @@ get_function_via_handler:
if (retval
&& !(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = get_scope(frame);
if (fcc->function_handler->common.scope != scope) {
if ((fcc->function_handler->common.fn_flags & ZEND_ACC_PRIVATE)
|| (!zend_check_protected(zend_get_function_root_class(fcc->function_handler), scope))) {
ZEND_ASSERT(!(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(fcc->function_handler, scope)) {
if (error) {
if (*error) {
efree(*error);
@ -4099,7 +4096,6 @@ get_function_via_handler:
}
}
}
}
} else if (error) {
if (fcc->calling_scope) {
zend_spprintf(error, 0, "class %s does not have a method \"%s\"", ZSTR_VAL(fcc->calling_scope->name), ZSTR_VAL(mname));

View file

@ -1098,12 +1098,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_inner(
} else {
fptr = zend_hash_find_ptr_lc(&ce->function_table, method_name);
if (fptr) {
if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) {
if (UNEXPECTED(fptr->common.scope != scope)) {
if (
UNEXPECTED(fptr->op_array.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fptr), scope))
) {
if (!zend_check_method_accessible(fptr, scope)) {
if (ce->__callstatic) {
zend_throw_error(NULL, "Creating a callable for the magic __callStatic() method is not supported in constant expressions");
} else {
@ -1112,8 +1107,6 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_inner(
return FAILURE;
}
}
}
} else {
if (ce->__callstatic) {
zend_throw_error(NULL, "Creating a callable for the magic __callStatic() method is not supported in constant expressions");

View file

@ -89,15 +89,10 @@ ZEND_FUNCTION(clone)
RETURN_THROWS();
}
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
if (clone && !zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
RETURN_THROWS();
}
}
}
zend_object *cloned;
cloned = zobj->handlers->clone_obj(zobj);
@ -953,13 +948,7 @@ ZEND_FUNCTION(get_class_methods)
scope = zend_get_executed_scope();
ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, mptr) {
if ((mptr->common.fn_flags & ZEND_ACC_PUBLIC)
|| (scope &&
(((mptr->common.fn_flags & ZEND_ACC_PROTECTED) &&
zend_check_protected(mptr->common.scope, scope))
|| ((mptr->common.fn_flags & ZEND_ACC_PRIVATE) &&
scope == mptr->common.scope)))
) {
if (zend_check_method_accessible(mptr, scope)) {
ZVAL_STR_COPY(&method_name, mptr->common.function_name);
zend_hash_next_index_insert_new(Z_ARRVAL_P(return_value), &method_name);
}

View file

@ -1949,11 +1949,10 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, zend_st
zval *func = zend_hash_find(&ce->function_table, lc_function_name);
if (EXPECTED(func)) {
fbc = Z_FUNC_P(func);
if (!(fbc->op_array.fn_flags & ZEND_ACC_PUBLIC)) {
if (!(fbc->common.fn_flags & ZEND_ACC_PUBLIC)) {
zend_class_entry *scope = zend_get_executed_scope();
if (UNEXPECTED(fbc->common.scope != scope)) {
if (UNEXPECTED(fbc->op_array.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fbc), scope))) {
ZEND_ASSERT(!(fbc->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(fbc, scope)) {
zend_function *fallback_fbc = get_static_method_fallback(ce, function_name);
if (!fallback_fbc) {
zend_bad_method_call(fbc, function_name, scope);
@ -1961,7 +1960,6 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, zend_st
fbc = fallback_fbc;
}
}
}
} else {
fbc = get_static_method_fallback(ce, function_name);
}
@ -2115,18 +2113,16 @@ ZEND_API zend_function *zend_std_get_constructor(zend_object *zobj) /* {{{ */
zend_function *constructor = zobj->ce->constructor;
if (constructor) {
if (UNEXPECTED(!(constructor->op_array.fn_flags & ZEND_ACC_PUBLIC))) {
if (UNEXPECTED(!(constructor->common.fn_flags & ZEND_ACC_PUBLIC))) {
zend_class_entry *scope = get_fake_or_executed_scope();
if (UNEXPECTED(constructor->common.scope != scope)) {
if (UNEXPECTED(constructor->op_array.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(constructor), scope))) {
ZEND_ASSERT(!(constructor->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(constructor, scope)) {
zend_bad_constructor_call(constructor, scope);
zend_object_store_ctor_failed(zobj);
constructor = NULL;
}
}
}
}
return constructor;
}

View file

@ -123,17 +123,15 @@ ZEND_API void zend_objects_destroy_object(zend_object *object)
zend_object *old_exception;
const zend_op *old_opline_before_exception;
if (destructor->op_array.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) {
if (destructor->op_array.fn_flags & ZEND_ACC_PRIVATE) {
/* Ensure that if we're calling a private function, we're allowed to do so.
*/
if (destructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) {
if (EG(current_execute_data)) {
zend_class_entry *scope = zend_get_executed_scope();
if (object->ce != scope) {
/* Ensure that if we're calling a protected or private function, we're allowed to do so. */
ZEND_ASSERT(!(destructor->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(destructor, scope)) {
zend_throw_error(NULL,
"Call to private %s::__destruct() from %s%s",
ZSTR_VAL(object->ce->name),
"Call to %s %s::__destruct() from %s%s",
zend_visibility_string(destructor->common.fn_flags), ZSTR_VAL(object->ce->name),
scope ? "scope " : "global scope",
scope ? ZSTR_VAL(scope->name) : ""
);
@ -141,32 +139,10 @@ ZEND_API void zend_objects_destroy_object(zend_object *object)
}
} else {
zend_error(E_WARNING,
"Call to private %s::__destruct() from global scope during shutdown ignored",
ZSTR_VAL(object->ce->name));
"Call to %s %s::__destruct() from global scope during shutdown ignored",
zend_visibility_string(destructor->common.fn_flags), ZSTR_VAL(object->ce->name));
return;
}
} else {
/* Ensure that if we're calling a protected function, we're allowed to do so.
*/
if (EG(current_execute_data)) {
zend_class_entry *scope = zend_get_executed_scope();
if (!zend_check_protected(zend_get_function_root_class(destructor), scope)) {
zend_throw_error(NULL,
"Call to protected %s::__destruct() from %s%s",
ZSTR_VAL(object->ce->name),
scope ? "scope " : "global scope",
scope ? ZSTR_VAL(scope->name) : ""
);
return;
}
} else {
zend_error(E_WARNING,
"Call to protected %s::__destruct() from global scope during shutdown ignored",
ZSTR_VAL(object->ce->name));
return;
}
}
}
GC_ADDREF(object);

View file

@ -137,5 +137,16 @@ static inline zend_property_info *zend_get_typed_property_info_for_slot(zend_obj
return NULL;
}
static zend_always_inline bool zend_check_method_accessible(const zend_function *fn, const zend_class_entry *scope)
{
if (!(fn->common.fn_flags & ZEND_ACC_PUBLIC)
&& fn->common.scope != scope
&& (UNEXPECTED(fn->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fn), scope)))) {
return false;
}
return true;
}
#endif /* ZEND_OBJECTS_H */

View file

@ -6043,16 +6043,14 @@ ZEND_VM_COLD_CONST_HANDLER(110, ZEND_CLONE, CONST|TMPVAR|UNUSED|THIS|CV, ANY)
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = EX(func)->op_array.scope;
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
FREE_OP1();
ZVAL_UNDEF(EX_VAR(opline->result.var));
HANDLE_EXCEPTION();
}
}
}
ZVAL_OBJ(EX_VAR(opline->result.var), clone_call(zobj));

24
Zend/zend_vm_execute.h generated
View file

@ -5217,16 +5217,14 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CLONE_SPEC_CONST_
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = EX(func)->op_array.scope;
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
ZVAL_UNDEF(EX_VAR(opline->result.var));
HANDLE_EXCEPTION();
}
}
}
ZVAL_OBJ(EX_VAR(opline->result.var), clone_call(zobj));
@ -15466,16 +15464,14 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CLONE_SPEC_TMPVAR_HANDLER(ZEND
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = EX(func)->op_array.scope;
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
zval_ptr_dtor_nogc(EX_VAR(opline->op1.var));
ZVAL_UNDEF(EX_VAR(opline->result.var));
HANDLE_EXCEPTION();
}
}
}
ZVAL_OBJ(EX_VAR(opline->result.var), clone_call(zobj));
@ -33563,16 +33559,14 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CLONE_SPEC_UNUSED_HANDLER(ZEND
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = EX(func)->op_array.scope;
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
ZVAL_UNDEF(EX_VAR(opline->result.var));
HANDLE_EXCEPTION();
}
}
}
ZVAL_OBJ(EX_VAR(opline->result.var), clone_call(zobj));
@ -41084,16 +41078,14 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CLONE_SPEC_CV_HANDLER(ZEND_OPC
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
scope = EX(func)->op_array.scope;
if (clone->common.scope != scope) {
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
if (!zend_check_method_accessible(clone, scope)) {
zend_bad_method_call(clone, clone->common.function_name, scope);
ZVAL_UNDEF(EX_VAR(opline->result.var));
HANDLE_EXCEPTION();
}
}
}
ZVAL_OBJ(EX_VAR(opline->result.var), clone_call(zobj));