Fixed bug #70630 (Closure::call/bind() crash with ReflectionFunction->getClosure())

This additionally removes support for binding to an unknown (not in parent hierarchy) scope.
Removing support for cross-scope is necessary for certain compile-time assumptions (like class constants) to prevent unexpected results

diff --git a/NEWS b/NEWS
index d641dddb87382b66af3aea70dc28949cceb958c7..9051f4d3338f74d1699032ce6a35173b93fc6a8a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 15 Oct 2015, PHP 7.0.0 RC 5
 
+- Core
+  . Fixed bug #70630 (Closure::call/bind() crash with
+    ReflectionFunction->getClosure()). (Bob)
+
 - Mcrypt:
   . Fixed bug #70625 (mcrypt_encrypt() won't return data when no IV was
     specified under RC4). (Nikita)

diff --git a/Zend/tests/bug70630.phpt b/Zend/tests/bug70630.phpt
new file mode 100644 (file)
index 0000000..3dabca6
--- /dev/null
+++ b/Zend/tests/bug70630.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #70630 (Closure::call/bind() crash with ReflectionFunction->getClosure())
+--FILE--
+<?php
+
+class a {}
+function foo() {}
+
+foreach (["substr", "foo"] as $fn) {
+       $x = (new ReflectionFunction($fn))->getClosure();
+       $x->call(new a);
+       Closure::bind($x, new a, "a");
+}
+
+?>
+--EXPECTF--
+
+Warning: Cannot bind function substr to an object in %s on line %d
+
+Warning: Cannot bind function substr to an object in %s on line %d
+
+Warning: Cannot bind function foo to an object in %s on line %d
+
+Warning: Cannot bind function foo to an object in %s on line %d

diff --git a/Zend/tests/closure_061.phpt b/Zend/tests/closure_061.phpt
new file mode 100644 (file)
index 0000000..a195956
--- /dev/null
+++ b/Zend/tests/closure_061.phpt
@@ -0,0 +1,64 @@
+--TEST--
+Closure::call() or Closure::bind() to independent class must fail
+--FILE--
+<?php
+
+class foo {
+       public $var;
+    
+       function initClass() {
+               $this->var = __CLASS__;
+       }
+}
+
+class bar {
+       public $var;
+    
+       function initClass() {
+               $this->var = __CLASS__;
+       }
+    
+       function getVar() {
+               assert($this->var !== null); // ensure initClass was called
+               return $this->var;
+       }
+}
+
+class baz extends bar {
+       public $var;
+    
+       function initClass() {
+               $this->var = __CLASS__;
+       }
+}
+
+function callMethodOn($class, $method, $object) {
+       $closure = (new ReflectionMethod($class, $method))->getClosure((new ReflectionClass($class))->newInstanceWithoutConstructor());
+       $closure = $closure->bindTo($object, $class);
+       return $closure();
+}
+
+$baz = new baz;
+
+callMethodOn("baz", "initClass", $baz);
+var_dump($baz->getVar());
+
+callMethodOn("bar", "initClass", $baz);
+var_dump($baz->getVar());
+
+callMethodOn("foo", "initClass", $baz);
+var_dump($baz->getVar());
+
+?>
+--EXPECTF--
+string(3) "baz"
+string(3) "bar"
+
+Warning: Cannot bind function foo::initClass to object of class baz in %s on line %d
+
+Fatal error: Uncaught Error: Using $this when not in object context in %s:%d
+Stack trace:
+#0 %s(%d): initClass()
+#1 %s(%d): callMethodOn('foo', 'initClass', Object(baz))
+#2 {main}
+  thrown in %s on line %d

diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index 50aa7d4188c12c46d336174079396cfd5894f2ab..8de7b6e488ee06dea06b2bbe0850ce70dd4886e0 100644 (file)
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -94,10 +94,12 @@ ZEND_METHOD(Closure, call)
                return;
        }
 
-       if (closure->func.type == ZEND_INTERNAL_FUNCTION) {
-               /* verify that we aren't binding internal function to a wrong object */
-               if ((closure->func.common.fn_flags & ZEND_ACC_STATIC) == 0 &&
-                               !instanceof_function(Z_OBJCE_P(newthis), closure->func.common.scope)) {
+       if (closure->func.type != ZEND_USER_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) {
+               /* verify that we aren't binding methods to a wrong object */
+               if (closure->func.common.scope == NULL) {
+                       zend_error(E_WARNING, "Cannot bind function %s to an object", ZSTR_VAL(closure->func.common.function_name));
+                       return;
+               } else if (!instanceof_function(Z_OBJCE_P(newthis), closure->func.common.scope)) {
                        zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(Z_OBJCE_P(newthis)->name));
                        return;
                }
@@ -165,7 +167,7 @@ ZEND_METHOD(Closure, bind)
                RETURN_NULL();
        }
 
-       closure = (zend_closure *)Z_OBJ_P(zclosure);
+       closure = (zend_closure *) Z_OBJ_P(zclosure);
 
        if ((newthis != NULL) && (closure->func.common.fn_flags & ZEND_ACC_STATIC)) {
                zend_error(E_WARNING, "Cannot bind an instance to a static closure");
@@ -187,7 +189,7 @@ ZEND_METHOD(Closure, bind)
                        }
                        zend_string_release(class_name);
                }
-               if(ce && ce != closure->func.common.scope && ce->type == ZEND_INTERNAL_CLASS) {
+               if (ce && ce != closure->func.common.scope && ce->type == ZEND_INTERNAL_CLASS) {
                        /* rebinding to internal class is not allowed */
                        zend_error(E_WARNING, "Cannot bind closure to scope of internal class %s", ZSTR_VAL(ce->name));
                        return;
@@ -202,6 +204,22 @@ ZEND_METHOD(Closure, bind)
                called_scope = ce;
        }
 
+       /* verify that we aren't binding methods to a wrong object */
+       if (closure->func.type != ZEND_USER_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) {
+               if (!closure->func.common.scope) {
+                       if (ce) {
+                               zend_error(E_WARNING, "Cannot bind function %s to an object", ZSTR_VAL(closure->func.common.function_name));
+                               return;
+                       }
+               } else if (!ce) {
+                       zend_error(E_WARNING, "Cannot bind function %s::%s to no class", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name));
+                       return;
+               } else if (!instanceof_function(ce, closure->func.common.scope)) {
+                       zend_error(E_WARNING, "Cannot bind function %s::%s to class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(ce->name));
+                       return;
+               }
+       }
+
        zend_create_closure(return_value, &closure->func, ce, called_scope, newthis);
        new_closure = (zend_closure *) Z_OBJ_P(return_value);
 
@@ -242,11 +260,9 @@ ZEND_API zend_function *zend_get_closure_invoke_method(zend_object *object) /* {
         * and we won't check arguments on internal function. We also set
         * ZEND_ACC_USER_ARG_INFO flag to prevent invalid usage by Reflection */
        invoke->type = ZEND_INTERNAL_FUNCTION;
-       invoke->internal_function.fn_flags =
-               ZEND_ACC_PUBLIC | ZEND_ACC_CALL_VIA_HANDLER | (closure->func.common.fn_flags & keep_flags);
+       invoke->internal_function.fn_flags = ZEND_ACC_PUBLIC | ZEND_ACC_CALL_VIA_HANDLER | (closure->func.common.fn_flags & keep_flags);
        if (closure->func.type != ZEND_INTERNAL_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_USER_ARG_INFO)) {
-               invoke->internal_function.fn_flags |=
-                       ZEND_ACC_USER_ARG_INFO;
+               invoke->internal_function.fn_flags |= ZEND_ACC_USER_ARG_INFO;
        }
        invoke->internal_function.handler = ZEND_MN(Closure___invoke);
        invoke->internal_function.module = 0;
@@ -523,10 +539,10 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
 
        object_init_ex(res, zend_ce_closure);
 
-       closure = (zend_closure *)Z_OBJ_P(res);
+       closure = (zend_closure *) Z_OBJ_P(res);
 
        memcpy(&closure->func, func, func->type == ZEND_USER_FUNCTION ? sizeof(zend_op_array) : sizeof(zend_internal_function));
-       closure->func.common.prototype = (zend_function*)closure;
+       closure->func.common.prototype = (zend_function *) closure;
        closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;
 
        if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) {
@@ -550,7 +566,8 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
                if (closure->func.op_array.refcount) {
                        (*closure->func.op_array.refcount)++;
                }
-       } else {
+       }
+       if (closure->func.type != ZEND_USER_FUNCTION || (func->common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) {
                /* verify that we aren't binding internal function to a wrong scope */
                if(func->common.scope != NULL) {
                        if(scope && !instanceof_function(scope, func->common.scope)) {
@@ -561,7 +578,6 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
                                        !instanceof_function(Z_OBJCE_P(this_ptr), closure->func.common.scope)) {
                                zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(Z_OBJCE_P(this_ptr)->name));
                                scope = NULL;
-                               this_ptr = NULL;
                        }
                } else {
                        /* if it's a free function, we won't set scope & this since they're meaningless */

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 98af0a7c7464036ccd2605fab30560fbc8264968..d30db31bd95b4706ec1cf92f734cc4b864e83de7 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -4854,7 +4854,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast) /* {{{ */
                op_array->doc_comment = zend_string_copy(decl->doc_comment);
        }
        if (decl->kind == ZEND_AST_CLOSURE) {
-               op_array->fn_flags |= ZEND_ACC_CLOSURE;
+               op_array->fn_flags |= ZEND_ACC_CLOSURE | ZEND_ACC_REAL_CLOSURE;
        }
 
        if (is_method) {

diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
index 68cc39ad8419e25559d4d048da2b9868ac5c1734..865340738aee10b574068360219a9e2e761dda7e 100644 (file)
--- a/Zend/zend_compile.h
+++ b/Zend/zend_compile.h
@@ -238,7 +238,7 @@ typedef struct _zend_try_catch_element {
 /* user class has methods with static variables */
 #define ZEND_HAS_STATIC_IN_METHODS    0x800000
 
-
+#define ZEND_ACC_REAL_CLOSURE         0x40
 #define ZEND_ACC_CLOSURE              0x100000
 #define ZEND_ACC_GENERATOR            0x800000