From 4f57b965ed50acd3ffbd3baaaffe7aa2660382bd Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 1 Dec 2022 11:02:37 +0100 Subject: [PATCH] Fix incorrect short-circuiting in constant expressions Fixes GH-10014 --- Zend/tests/gh10014.phpt | 14 ++++++++ Zend/zend_ast.c | 75 +++++++++++++++++++++++------------------ Zend/zend_ast.h | 3 +- Zend/zend_execute_API.c | 3 +- 4 files changed, 59 insertions(+), 36 deletions(-) create mode 100644 Zend/tests/gh10014.phpt diff --git a/Zend/tests/gh10014.phpt b/Zend/tests/gh10014.phpt new file mode 100644 index 0000000000000..0870c5631336a --- /dev/null +++ b/Zend/tests/gh10014.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-10014: Incorrect short-circuiting in constant expressions +--FILE-- +y]->y)] +class y { +} + +?> +--EXPECTF-- +Warning: Undefined array key 2 in %s on line %d + +Warning: Attempt to read property "y" on array in %s on line %d diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index b007c08c0e56e..eea44eaa81c06 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -498,17 +498,23 @@ zend_class_entry *zend_ast_fetch_class(zend_ast *ast, zend_class_entry *scope) return zend_fetch_class_with_scope(zend_ast_get_str(ast), (ast->attr >> ZEND_CONST_EXPR_NEW_FETCH_TYPE_SHIFT) | ZEND_FETCH_CLASS_EXCEPTION, scope); } -ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx) -{ +ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex( + zval *result, + zend_ast *ast, + zend_class_entry *scope, + bool *short_circuited_ptr, + zend_ast_evaluate_ctx *ctx +) { zval op1, op2; zend_result ret = SUCCESS; - ctx->short_circuited = false; + bool short_circuited; + *short_circuited_ptr = false; switch (ast->kind) { case ZEND_AST_BINARY_OP: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; - } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; } else { @@ -520,9 +526,9 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * break; case ZEND_AST_GREATER: case ZEND_AST_GREATER_EQUAL: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; - } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; } else { @@ -535,7 +541,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * } break; case ZEND_AST_UNARY_OP: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; } else { unary_op_type op = get_unary_op(ast->attr); @@ -588,12 +594,12 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * } break; case ZEND_AST_AND: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; break; } if (zend_is_true(&op1)) { - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -606,14 +612,14 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * zval_ptr_dtor_nogc(&op1); break; case ZEND_AST_OR: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; break; } if (zend_is_true(&op1)) { ZVAL_TRUE(result); } else { - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -624,7 +630,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * zval_ptr_dtor_nogc(&op1); break; case ZEND_AST_CONDITIONAL: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; break; } @@ -632,7 +638,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * if (!ast->child[1]) { *result = op1; } else { - if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -640,7 +646,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * zval_ptr_dtor_nogc(&op1); } } else { - if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[2], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[2], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -649,14 +655,14 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * } break; case ZEND_AST_COALESCE: - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; break; } if (Z_TYPE(op1) > IS_NULL) { *result = op1; } else { - if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -665,7 +671,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * } break; case ZEND_AST_UNARY_PLUS: - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; } else { ZVAL_LONG(&op1, 0); @@ -674,7 +680,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * } break; case ZEND_AST_UNARY_MINUS: - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; } else { ZVAL_LONG(&op1, 0); @@ -695,7 +701,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * for (i = 0; i < list->children; i++) { zend_ast *elem = list->child[i]; if (elem->kind == ZEND_AST_UNPACK) { - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[0], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(result); return FAILURE; } @@ -708,14 +714,14 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * continue; } if (elem->child[1]) { - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(result); return FAILURE; } } else { ZVAL_UNDEF(&op1); } - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, elem->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, elem->child[0], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); zval_ptr_dtor_nogc(result); return FAILURE; @@ -734,11 +740,12 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * zend_error_noreturn(E_COMPILE_ERROR, "Cannot use [] for reading"); } - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { ret = FAILURE; break; } - if (ctx->short_circuited) { + if (short_circuited) { + *short_circuited_ptr = true; ZVAL_NULL(result); return SUCCESS; } @@ -751,7 +758,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * break; } - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -785,7 +792,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * zval case_value_zv; ZVAL_UNDEF(&case_value_zv); if (case_value_ast != NULL) { - if (UNEXPECTED(zend_ast_evaluate_ex(&case_value_zv, case_value_ast, scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&case_value_zv, case_value_ast, scope, &short_circuited, ctx) != SUCCESS)) { return FAILURE; } } @@ -847,7 +854,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * name = zend_ast_get_str(arg_ast->child[0]); arg_ast = arg_ast->child[1]; } - if (zend_ast_evaluate_ex(&arg, arg_ast, scope, ctx) == FAILURE) { + if (zend_ast_evaluate_ex(&arg, arg_ast, scope, &short_circuited, ctx) == FAILURE) { zend_array_destroy(args); zval_ptr_dtor(result); return FAILURE; @@ -877,7 +884,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * ALLOCA_FLAG(use_heap) zval *args = do_alloca(sizeof(zval) * args_ast->children, use_heap); for (uint32_t i = 0; i < args_ast->children; i++) { - if (zend_ast_evaluate_ex(&args[i], args_ast->child[i], scope, ctx) == FAILURE) { + if (zend_ast_evaluate_ex(&args[i], args_ast->child[i], scope, &short_circuited, ctx) == FAILURE) { for (uint32_t j = 0; j < i; j++) { zval_ptr_dtor(&args[j]); } @@ -909,20 +916,21 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * case ZEND_AST_PROP: case ZEND_AST_NULLSAFE_PROP: { - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited, ctx) != SUCCESS)) { return FAILURE; } - if (ctx->short_circuited) { + if (short_circuited) { + *short_circuited_ptr = true; ZVAL_NULL(result); return SUCCESS; } if (ast->kind == ZEND_AST_NULLSAFE_PROP && Z_TYPE(op1) == IS_NULL) { - ctx->short_circuited = true; + *short_circuited_ptr = true; ZVAL_NULL(result); return SUCCESS; } - if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, &short_circuited, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); return FAILURE; } @@ -976,7 +984,8 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast * ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope) { zend_ast_evaluate_ctx ctx = {0}; - return zend_ast_evaluate_ex(result, ast, scope, &ctx); + bool short_circuited; + return zend_ast_evaluate_ex(result, ast, scope, &short_circuited, &ctx); } static size_t ZEND_FASTCALL zend_ast_tree_size(zend_ast *ast) diff --git a/Zend/zend_ast.h b/Zend/zend_ast.h index cf455ef50c974..77c277f960115 100644 --- a/Zend/zend_ast.h +++ b/Zend/zend_ast.h @@ -300,11 +300,10 @@ ZEND_API zend_ast *zend_ast_create_decl( typedef struct { bool had_side_effects; - bool short_circuited; } zend_ast_evaluate_ctx; ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope); -ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx); +ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, bool *short_circuited_ptr, zend_ast_evaluate_ctx *ctx); ZEND_API zend_string *zend_ast_export(const char *prefix, zend_ast *ast, const char *suffix); ZEND_API zend_ast_ref * ZEND_FASTCALL zend_ast_copy(zend_ast *ast); diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index 8f993190ab178..8dfffb48060d0 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -686,8 +686,9 @@ ZEND_API zend_result ZEND_FASTCALL zval_update_constant_with_ctx(zval *p, zend_c ZVAL_COPY_OR_DUP(p, zv); } else { zval tmp; + bool short_circuited; - if (UNEXPECTED(zend_ast_evaluate_ex(&tmp, ast, scope, ctx) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&tmp, ast, scope, &short_circuited, ctx) != SUCCESS)) { return FAILURE; } zval_ptr_dtor_nogc(p);