diff --git a/Zend/tests/closures/closure_const_expr/attributes.phpt b/Zend/tests/closures/closure_const_expr/attributes.phpt index c83b821d12969..e331bc2edd568 100644 --- a/Zend/tests/closures/closure_const_expr/attributes.phpt +++ b/Zend/tests/closures/closure_const_expr/attributes.phpt @@ -8,13 +8,13 @@ reflection #[Attribute(Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)] class Attr { public function __construct(public Closure $value) { - $value('foo'); + $value('foo'); } } #[Attr(static function () { })] #[Attr(static function (...$args) { - var_dump($args); + var_dump($args); })] class C {} diff --git a/Zend/tests/closures/closure_const_expr/attributes_autoload.inc b/Zend/tests/closures/closure_const_expr/attributes_autoload.inc new file mode 100644 index 0000000000000..48f9b1987cf5d --- /dev/null +++ b/Zend/tests/closures/closure_const_expr/attributes_autoload.inc @@ -0,0 +1,9 @@ + diff --git a/Zend/tests/closures/closure_const_expr/attributes_autoload.phpt b/Zend/tests/closures/closure_const_expr/attributes_autoload.phpt new file mode 100644 index 0000000000000..fa99e8b5abaa3 --- /dev/null +++ b/Zend/tests/closures/closure_const_expr/attributes_autoload.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-17851: Use-after-free when instantiating autoloaded class with attribute having a Closure parameter. +--EXTENSIONS-- +reflection +--FILE-- +getAttributes() as $reflectionAttribute) { + var_dump($reflectionAttribute->newInstance()); +} + +?> +--EXPECTF-- +object(Attr)#%d (1) { + ["value"]=> + object(Closure)#%d (3) { + ["name"]=> + string(%d) "{closure:%s:%d}" + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + } +} +array(1) { + [0]=> + string(3) "foo" +} +object(Attr)#%d (1) { + ["value"]=> + object(Closure)#%d (4) { + ["name"]=> + string(%d) "{closure:%s:%d}" + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + ["parameter"]=> + array(1) { + ["$args"]=> + string(10) "" + } + } +} diff --git a/Zend/tests/closures/closure_const_expr/static_variable.phpt b/Zend/tests/closures/closure_const_expr/static_variable.phpt new file mode 100644 index 0000000000000..5aea594d8620b --- /dev/null +++ b/Zend/tests/closures/closure_const_expr/static_variable.phpt @@ -0,0 +1,77 @@ +--TEST-- +Closures in const expressions support static variables. +--FILE-- + +--EXPECTF-- +object(Closure)#%d (4) { + ["name"]=> + string(%d) "{closure:%s:%d}" + ["file"]=> + string(%d) "%s" + ["line"]=> + int(3) + ["static"]=> + array(2) { + ["x"]=> + array(0) { + } + ["i"]=> + int(1) + } +} +array(1) { + [0]=> + int(2) +} +array(2) { + [0]=> + int(2) + [1]=> + int(4) +} +array(3) { + [0]=> + int(2) + [1]=> + int(4) + [2]=> + int(8) +} +object(Closure)#%d (4) { + ["name"]=> + string(%d) "{closure:%s:%d}" + ["file"]=> + string(%d) "%s" + ["line"]=> + int(3) + ["static"]=> + array(2) { + ["x"]=> + array(3) { + [0]=> + int(2) + [1]=> + int(4) + [2]=> + int(8) + } + ["i"]=> + int(8) + } +} diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 1ae7a0ef7a68e..2551f876d4465 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1277,6 +1277,7 @@ static void* ZEND_FASTCALL zend_ast_tree_copy(zend_ast *ast, void *buf) new->attr = old->attr; new->lineno = old->lineno; new->op_array = old->op_array; + function_add_ref((zend_function *)new->op_array); buf = (void*)((char*)buf + sizeof(zend_ast_op_array)); } else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) { zend_ast_fcc *old = (zend_ast_fcc*)ast; @@ -1353,7 +1354,7 @@ ZEND_API void ZEND_FASTCALL zend_ast_destroy(zend_ast *ast) } else if (EXPECTED(ast->kind == ZEND_AST_CONSTANT)) { zend_string_release_ex(zend_ast_get_constant_name(ast), 0); } else if (EXPECTED(ast->kind == ZEND_AST_OP_ARRAY)) { - /* Nothing to do. */ + destroy_op_array(zend_ast_get_op_array(ast)->op_array); } else if (EXPECTED(zend_ast_is_decl(ast))) { zend_ast_decl *decl = (zend_ast_decl *) ast; diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index e9945b3284cd9..5777e1a34a2b8 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -528,7 +528,6 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */ /* We don't own the static variables of fake closures. */ if (!(closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) { zend_destroy_static_vars(&closure->func.op_array); - closure->func.op_array.static_variables = NULL; } destroy_op_array(&closure->func.op_array); } else if (closure->func.type == ZEND_INTERNAL_FUNCTION) { @@ -760,16 +759,14 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en } /* For fake closures, we want to reuse the static variables of the original function. */ + HashTable *ht = ZEND_MAP_PTR_GET(func->op_array.static_variables_ptr); if (!is_fake) { - if (closure->func.op_array.static_variables) { - closure->func.op_array.static_variables = - zend_array_dup(closure->func.op_array.static_variables); + if (!ht) { + ht = closure->func.op_array.static_variables; } ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr, - closure->func.op_array.static_variables); + ht ? zend_array_dup(ht) : NULL); } else if (func->op_array.static_variables) { - HashTable *ht = ZEND_MAP_PTR_GET(func->op_array.static_variables_ptr); - if (!ht) { ht = zend_array_dup(func->op_array.static_variables); ZEND_MAP_PTR_SET(func->op_array.static_variables_ptr, ht); diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 1cba9ef221997..c2724955f17b8 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8244,9 +8244,6 @@ static zend_string *zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_register_seen_symbol(lcname, ZEND_SYMBOL_FUNCTION); switch (level) { - case FUNC_DECL_LEVEL_CONSTEXPR: - zend_add_dynamic_func_def(op_array); - break; case FUNC_DECL_LEVEL_NESTED: { uint32_t func_ref = zend_add_dynamic_func_def(op_array); if (op_array->fn_flags & ZEND_ACC_CLOSURE) { @@ -8261,6 +8258,7 @@ static zend_string *zend_begin_func_decl(znode *result, zend_op_array *op_array, } break; } + case FUNC_DECL_LEVEL_CONSTEXPR: case FUNC_DECL_LEVEL_TOPLEVEL: /* Nothing to do. */ break; diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index f32ae13e06793..ce052024ae7a0 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -636,13 +636,6 @@ ZEND_API void destroy_op_array(zend_op_array *op_array) } if (op_array->num_dynamic_func_defs) { for (i = 0; i < op_array->num_dynamic_func_defs; i++) { - /* Closures overwrite static_variables in their copy. - * Make sure to destroy them when the prototype function is destroyed. */ - if (op_array->dynamic_func_defs[i]->static_variables - && (op_array->dynamic_func_defs[i]->fn_flags & ZEND_ACC_CLOSURE)) { - zend_array_destroy(op_array->dynamic_func_defs[i]->static_variables); - op_array->dynamic_func_defs[i]->static_variables = NULL; - } destroy_op_array(op_array->dynamic_func_defs[i]); } efree(op_array->dynamic_func_defs); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index e612dcd80748b..a2346a56d458e 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -242,6 +242,15 @@ static void zend_file_cache_unserialize_zval(zval *zv, zend_persistent_script *script, void *buf); +static void zend_file_cache_serialize_func(zval *zv, + zend_persistent_script *script, + zend_file_cache_metainfo *info, + void *buf); + +static void zend_file_cache_unserialize_func(zval *zv, + zend_persistent_script *script, + void *buf); + static void *zend_file_cache_serialize_interned(zend_string *str, zend_file_cache_metainfo *info) { @@ -364,8 +373,10 @@ static void zend_file_cache_serialize_ast(zend_ast *ast, } } } else if (ast->kind == ZEND_AST_OP_ARRAY) { - /* The op_array itself will be serialized as part of the dynamic_func_defs. */ - SERIALIZE_PTR(zend_ast_get_op_array(ast)->op_array); + zval z; + ZVAL_PTR(&z, zend_ast_get_op_array(ast)->op_array); + zend_file_cache_serialize_func(&z, script, info, buf); + zend_ast_get_op_array(ast)->op_array = Z_PTR(z); } else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) { zend_ast_fcc *fcc = (zend_ast_fcc*)ast; ZEND_MAP_PTR_INIT(fcc->fptr, NULL); @@ -1252,8 +1263,10 @@ static void zend_file_cache_unserialize_ast(zend_ast *ast, } } } else if (ast->kind == ZEND_AST_OP_ARRAY) { - /* The op_array itself will be unserialized as part of the dynamic_func_defs. */ - UNSERIALIZE_PTR(zend_ast_get_op_array(ast)->op_array); + zval z; + ZVAL_PTR(&z, zend_ast_get_op_array(ast)->op_array); + zend_file_cache_unserialize_func(&z, script, buf); + zend_ast_get_op_array(ast)->op_array = Z_PTR(z); } else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) { zend_ast_fcc *fcc = (zend_ast_fcc*)ast; ZEND_MAP_PTR_NEW(fcc->fptr);