From be426611c019d80239e45ca902d77010d18b4c0b Mon Sep 17 00:00:00 2001 From: Reeze Xia Date: Wed, 6 Mar 2013 15:25:42 +0800 Subject: [PATCH 1/3] Fixed bug #61025 (__invoke() visibility not honored) - check visibility of __invoke when calling - make is_callable consist - Add the ZEND_ACC_CLOSURE flag to the closure created with: zend_create_closure() --- Zend/tests/bug61025-1.phpt | 17 ++++++++++ Zend/tests/bug61025-2.phpt | 67 ++++++++++++++++++++++++++++++++++++++ Zend/tests/bug61025.phpt | 34 +++++++++++++++++++ Zend/zend_API.c | 11 +++++++ Zend/zend_closures.c | 1 + Zend/zend_vm_def.h | 11 +++++++ Zend/zend_vm_execute.h | 44 +++++++++++++++++++++++++ 7 files changed, 185 insertions(+) create mode 100644 Zend/tests/bug61025-1.phpt create mode 100644 Zend/tests/bug61025-2.phpt create mode 100644 Zend/tests/bug61025.phpt diff --git a/Zend/tests/bug61025-1.phpt b/Zend/tests/bug61025-1.phpt new file mode 100644 index 000000000000..42291f89ce47 --- /dev/null +++ b/Zend/tests/bug61025-1.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #61025 (__invoke() visibility not honored) invoke private +--FILE-- + +===DONE=== +--EXPECTF-- +Fatal error: Call to private FooPrivate::__invoke() from context '' in %s/bug61025-1.php on line %d \ No newline at end of file diff --git a/Zend/tests/bug61025-2.phpt b/Zend/tests/bug61025-2.phpt new file mode 100644 index 000000000000..dc0ab95e20da --- /dev/null +++ b/Zend/tests/bug61025-2.phpt @@ -0,0 +1,67 @@ +--TEST-- +Bug #61025 (__invoke() visibility not honored) is_callable consist +--FILE-- + +===DONE=== +--EXPECT-- +Check in class: FooPublic $this is callable: true +Check outter is callable: true + +Check in class: FooProtected $this is callable: true +Check outter is callable: false + +Check in class: FooPrivate $this is callable: true +Check outter is callable: false + +===DONE=== \ No newline at end of file diff --git a/Zend/tests/bug61025.phpt b/Zend/tests/bug61025.phpt new file mode 100644 index 000000000000..8ea9b1b60a16 --- /dev/null +++ b/Zend/tests/bug61025.phpt @@ -0,0 +1,34 @@ +--TEST-- +Bug #61025 (__invoke() visibility not honored) +--FILE-- + +===DONE=== +--EXPECTF-- +Closure +FooPublic::__invoke() + +Fatal error: Call to protected FooProtected::__invoke() from context '' in %s/bug61025.php on line %d \ No newline at end of file diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 95c90ea753c8..88dad92810da 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -3100,6 +3100,17 @@ ZEND_API zend_bool zend_is_callable_ex(zval *callable, zval *object_ptr, uint ch case IS_OBJECT: if (Z_OBJ_HANDLER_P(callable, get_closure) && Z_OBJ_HANDLER_P(callable, get_closure)(callable, &fcc->calling_scope, &fcc->function_handler, &fcc->object_ptr TSRMLS_CC) == SUCCESS) { fcc->called_scope = fcc->calling_scope; + if (!(fcc->function_handler->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (fcc->function_handler->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(fcc->called_scope != EG(scope))) { + return 0; + } + } else if ((fcc->function_handler->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fcc->function_handler), EG(scope)))) { + return 0; + } + } + } if (callable_name) { zend_class_entry *ce = Z_OBJCE_P(callable); /* TBFixed: what if it's overloaded? */ diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 0de928333770..f1640bc117ac 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -450,6 +450,7 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure->func = *func; closure->func.common.prototype = NULL; + closure->func.common.fn_flags |= ZEND_ACC_CLOSURE; if ((scope == NULL) && (this_ptr != NULL)) { /* use dummy scope if we're binding an object without specifying a scope */ diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 578a319fd192..0857504d844c 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2673,6 +2673,17 @@ ZEND_VM_HANDLER(59, ZEND_INIT_FCALL_BY_NAME, ANY, CONST|TMP|VAR|CV) EXPECTED(Z_TYPE_P(function_name) == IS_OBJECT) && Z_OBJ_HANDLER_P(function_name, get_closure) && Z_OBJ_HANDLER_P(function_name, get_closure)(function_name, &call->called_scope, &call->fbc, &call->object TSRMLS_CC) == SUCCESS) { + if (!(call->fbc->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (call->fbc->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(call->called_scope != EG(scope))) { + zend_error_noreturn(E_ERROR, "Call to private %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } else if ((call->fbc->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(call->fbc), EG(scope)))) { + zend_error_noreturn(E_ERROR, "Call to protected %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } + } if (call->object) { Z_ADDREF_P(call->object); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 392569678dff..1b0bc29e3b12 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1256,6 +1256,17 @@ static int ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME_SPEC_CONST_HANDLER(ZEND_OPCODE EXPECTED(Z_TYPE_P(function_name) == IS_OBJECT) && Z_OBJ_HANDLER_P(function_name, get_closure) && Z_OBJ_HANDLER_P(function_name, get_closure)(function_name, &call->called_scope, &call->fbc, &call->object TSRMLS_CC) == SUCCESS) { + if (!(call->fbc->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (call->fbc->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(call->called_scope != EG(scope))) { + zend_error_noreturn(E_ERROR, "Call to private %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } else if ((call->fbc->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(call->fbc), EG(scope)))) { + zend_error_noreturn(E_ERROR, "Call to protected %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } + } if (call->object) { Z_ADDREF_P(call->object); } @@ -1583,6 +1594,17 @@ static int ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME_SPEC_TMP_HANDLER(ZEND_OPCODE_H EXPECTED(Z_TYPE_P(function_name) == IS_OBJECT) && Z_OBJ_HANDLER_P(function_name, get_closure) && Z_OBJ_HANDLER_P(function_name, get_closure)(function_name, &call->called_scope, &call->fbc, &call->object TSRMLS_CC) == SUCCESS) { + if (!(call->fbc->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (call->fbc->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(call->called_scope != EG(scope))) { + zend_error_noreturn(E_ERROR, "Call to private %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } else if ((call->fbc->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(call->fbc), EG(scope)))) { + zend_error_noreturn(E_ERROR, "Call to protected %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } + } if (call->object) { Z_ADDREF_P(call->object); } @@ -1770,6 +1792,17 @@ static int ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME_SPEC_VAR_HANDLER(ZEND_OPCODE_H EXPECTED(Z_TYPE_P(function_name) == IS_OBJECT) && Z_OBJ_HANDLER_P(function_name, get_closure) && Z_OBJ_HANDLER_P(function_name, get_closure)(function_name, &call->called_scope, &call->fbc, &call->object TSRMLS_CC) == SUCCESS) { + if (!(call->fbc->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (call->fbc->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(call->called_scope != EG(scope))) { + zend_error_noreturn(E_ERROR, "Call to private %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } else if ((call->fbc->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(call->fbc), EG(scope)))) { + zend_error_noreturn(E_ERROR, "Call to protected %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } + } if (call->object) { Z_ADDREF_P(call->object); } @@ -1995,6 +2028,17 @@ static int ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME_SPEC_CV_HANDLER(ZEND_OPCODE_HA EXPECTED(Z_TYPE_P(function_name) == IS_OBJECT) && Z_OBJ_HANDLER_P(function_name, get_closure) && Z_OBJ_HANDLER_P(function_name, get_closure)(function_name, &call->called_scope, &call->fbc, &call->object TSRMLS_CC) == SUCCESS) { + if (!(call->fbc->common.fn_flags & (ZEND_ACC_PUBLIC | ZEND_ACC_CLOSURE))) { + if (call->fbc->common.fn_flags & ZEND_ACC_PRIVATE) { + if (UNEXPECTED(call->called_scope != EG(scope))) { + zend_error_noreturn(E_ERROR, "Call to private %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } else if ((call->fbc->common.fn_flags & ZEND_ACC_PROTECTED)) { + if (UNEXPECTED(!zend_check_protected(zend_get_function_root_class(call->fbc), EG(scope)))) { + zend_error_noreturn(E_ERROR, "Call to protected %s::__invoke() from context '%s'", Z_OBJCE_P(function_name)->name, EG(scope) ? EG(scope)->name : ""); + } + } + } if (call->object) { Z_ADDREF_P(call->object); } From e60ee2da830c1d9a3c95973e1d161913b06b5e50 Mon Sep 17 00:00:00 2001 From: Reeze Xia Date: Thu, 7 Mar 2013 00:47:48 +0800 Subject: [PATCH 2/3] __invoke() magic method should not be declared as static Related to bug #61025 Consist with similar magic methods which will raise warning when modified with improper modifiers --- Zend/zend_closures.c | 1 + Zend/zend_closures.h | 2 -- Zend/zend_compile.c | 9 +++++++++ Zend/zend_compile.h | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index f1640bc117ac..2bf2260045ac 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -28,6 +28,7 @@ #include "zend_objects.h" #include "zend_objects_API.h" #include "zend_globals.h" +#include "zend_compile.h" #define ZEND_CLOSURE_PRINT_NAME "Closure object" diff --git a/Zend/zend_closures.h b/Zend/zend_closures.h index c41cf475603f..9d50641d2198 100644 --- a/Zend/zend_closures.h +++ b/Zend/zend_closures.h @@ -24,8 +24,6 @@ BEGIN_EXTERN_C() -#define ZEND_INVOKE_FUNC_NAME "__invoke" - void zend_register_closure_ce(TSRMLS_D); extern ZEND_API zend_class_entry *zend_ce_closure; diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index e45419d3f0c8..a3ae0f32d165 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -29,6 +29,7 @@ #include "tsrm_virtual_cwd.h" #include "zend_multibyte.h" #include "zend_language_scanner.h" +#include "zend_closures.h" #define CONSTANT_EX(op_array, op) \ (op_array)->literals[op].constant @@ -1621,6 +1622,10 @@ void zend_do_begin_function_declaration(znode *function_token, znode *function_n if (fn_flags & ((ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC) ^ ZEND_ACC_PUBLIC)) { zend_error(E_WARNING, "The magic method __toString() must have public visibility and cannot be static"); } + } else if ((name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && (!memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1))) { + if (fn_flags & ZEND_ACC_STATIC) { + zend_error(E_WARNING, "The magic method __invoke() cannot be static"); + } } } else { char *class_lcname; @@ -1677,6 +1682,10 @@ void zend_do_begin_function_declaration(znode *function_token, znode *function_n zend_error(E_WARNING, "The magic method __toString() must have public visibility and cannot be static"); } CG(active_class_entry)->__tostring = (zend_function *) CG(active_op_array); + } else if ((name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && (!memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1))) { + if (fn_flags & ZEND_ACC_STATIC) { + zend_error(E_WARNING, "The magic method __invoke() cannot be static"); + } } else if (!(fn_flags & ZEND_ACC_STATIC)) { CG(active_op_array)->fn_flags |= ZEND_ACC_ALLOW_STATIC; } diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 8042dd54eee8..4d61ce24d5f6 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -856,6 +856,7 @@ END_EXTERN_C() #define ZEND_CALLSTATIC_FUNC_NAME "__callstatic" #define ZEND_TOSTRING_FUNC_NAME "__tostring" #define ZEND_AUTOLOAD_FUNC_NAME "__autoload" +#define ZEND_INVOKE_FUNC_NAME "__invoke" /* The following constants may be combined in CG(compiler_options) * to change the default compiler behavior */ From cd55a59a7640d299782f746632c12cace43d3296 Mon Sep 17 00:00:00 2001 From: Reeze Xia Date: Thu, 7 Mar 2013 01:03:07 +0800 Subject: [PATCH 3/3] Add missing test file for bug #61025 --- Zend/tests/bug61025-3.phpt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Zend/tests/bug61025-3.phpt diff --git a/Zend/tests/bug61025-3.phpt b/Zend/tests/bug61025-3.phpt new file mode 100644 index 000000000000..436081569bea --- /dev/null +++ b/Zend/tests/bug61025-3.phpt @@ -0,0 +1,21 @@ +--TEST-- +Bug #61025 (__invoke() visibility not honored) raise warning when using static +--FILE-- + +===DONE=== +--EXPECTF-- +Warning: The magic method __invoke() cannot be static in %s/bug61025-3.php on line %d + +Warning: The magic method __invoke() cannot be static in %s/bug61025-3.php on line %d +===DONE=== \ No newline at end of file