From 988bbe719fc5d6ff66db9dff769ecde0e97dabf2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 15 Apr 2024 21:15:28 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20GH-13970:=20Incorrect=20validation=20of?= =?UTF-8?q?=20#[\Attribute]=E2=80=99s=20first=20parameter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In c1959e63e5 code was added to deal with preloading of enums, which had to be special cased because they are object instances. In 4397811db2 this code was changed because array instances can contain object instances due to enums. In that commit, the code started using zval_update_constant_ex() and stopped ZEND_AST_CONST_ENUM_INIT resolution when in compilation. Since the attribute in the testcase is resolved during compilation, the resolution fails as well, even though in this case we can resolve it fine as the problem is only with constants during preloading. This patch restores the original code but changes the IS_OBJECT condition to Z_COLLECTABLE such that both arrays and objects that are resolved from IS_CONSTANT_AST are not persisted to shm. --- Zend/zend_ast.c | 12 ++++++------ ext/opcache/ZendAccelerator.c | 19 ++++++++++++++++--- ext/zend_test/tests/gh13970.phpt | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 ext/zend_test/tests/gh13970.phpt diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 2e4edeee5d98b..5edd4c4d37ed6 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -769,12 +769,6 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as break; case ZEND_AST_CONST_ENUM_INIT: { - // Preloading will attempt to resolve constants but objects can't be stored in shm - // Aborting here to store the const AST instead - if (CG(in_compilation)) { - return FAILURE; - } - zend_ast *class_name_ast = ast->child[0]; zend_string *class_name = zend_ast_get_str(class_name_ast); @@ -792,6 +786,12 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as } zend_class_entry *ce = zend_lookup_class(class_name); + if (!ce) { + /* Class may not be available when resolving constants on a dynamically + * declared enum during preloading. */ + ZEND_ASSERT(CG(compiler_options) & ZEND_COMPILE_PRELOAD); + return FAILURE; + } zend_enum_new(result, ce, case_name, case_value_ast != NULL ? &case_value_zv : NULL); zval_ptr_dtor_nogc(&case_value_zv); break; diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 9bcd035c35295..2d0586ccef875 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -3755,6 +3755,19 @@ static zend_result preload_resolve_deps(preload_error *error, const zend_class_e return SUCCESS; } +static zend_result preload_update_constant(zval *val, zend_class_entry *scope) +{ + zval tmp; + ZVAL_COPY(&tmp, val); + if (zval_update_constant_ex(&tmp, scope) == FAILURE || Z_COLLECTABLE(tmp)) { + zval_ptr_dtor(&tmp); + return FAILURE; + } + zval_ptr_dtor_nogc(val); + ZVAL_COPY_VALUE(val, &tmp); + return SUCCESS; +} + static bool preload_try_resolve_constants(zend_class_entry *ce) { bool ok, changed, was_changed = false; @@ -3768,7 +3781,7 @@ static bool preload_try_resolve_constants(zend_class_entry *ce) ZEND_HASH_MAP_FOREACH_PTR(&ce->constants_table, c) { val = &c->value; if (Z_TYPE_P(val) == IS_CONSTANT_AST) { - if (EXPECTED(zval_update_constant_ex(val, c->ce) == SUCCESS)) { + if (EXPECTED(preload_update_constant(val, c->ce) == SUCCESS)) { was_changed = changed = true; } else { ok = false; @@ -3786,7 +3799,7 @@ static bool preload_try_resolve_constants(zend_class_entry *ce) val = &ce->default_properties_table[i]; if (Z_TYPE_P(val) == IS_CONSTANT_AST) { zend_property_info *prop = ce->properties_info_table[i]; - if (UNEXPECTED(zval_update_constant_ex(val, prop->ce) != SUCCESS)) { + if (UNEXPECTED(preload_update_constant(val, prop->ce) != SUCCESS)) { resolved = ok = false; } } @@ -3802,7 +3815,7 @@ static bool preload_try_resolve_constants(zend_class_entry *ce) val = ce->default_static_members_table + ce->default_static_members_count - 1; while (count) { if (Z_TYPE_P(val) == IS_CONSTANT_AST) { - if (UNEXPECTED(zval_update_constant_ex(val, ce) != SUCCESS)) { + if (UNEXPECTED(preload_update_constant(val, ce) != SUCCESS)) { resolved = ok = false; } } diff --git a/ext/zend_test/tests/gh13970.phpt b/ext/zend_test/tests/gh13970.phpt new file mode 100644 index 0000000000000..8a52f3f47c8c2 --- /dev/null +++ b/ext/zend_test/tests/gh13970.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-13970 (Incorrect validation of #[\Attribute]’s first parameter) +--EXTENSIONS-- +zend_test +--FILE-- +getAttributes()[0]->newInstance()); +?> +--EXPECTF-- +Fatal error: Attribute::__construct(): Argument #1 ($flags) must be of type int, ZendTestUnitEnum given in %s on line %d