]> granicus.if.org Git - php/commitdiff
Fix constant evaluation of && and ||
authorNikita Popov <nikita.ppv@gmail.com>
Fri, 6 Dec 2019 10:07:57 +0000 (11:07 +0100)
committerNikita Popov <nikita.ppv@gmail.com>
Fri, 6 Dec 2019 10:07:57 +0000 (11:07 +0100)
The "return" in the for loop should have been a break on the switch,
otherwise the result is just ignored... but because it prevents
evaluation of the other operand, it also violates the invariant that
everything has been constant evaluated, resulting in an assertion
failure.

The for loop isn't correct in any case though, because it's not legal
to determine the result based on just the second operand, as the
first one may have a side-effect that cannot be optimized away.

Zend/tests/const_eval_and.phpt [new file with mode: 0644]
Zend/zend_compile.c

diff --git a/Zend/tests/const_eval_and.phpt b/Zend/tests/const_eval_and.phpt
new file mode 100644 (file)
index 0000000..90917ba
--- /dev/null
@@ -0,0 +1,9 @@
+--TEST--
+Incorrect constant evaluation of and/or (OSS-Fuzz #19255)
+--FILE--
+<?php
+const C = 0 && __namespace__;
+var_dump(C);
+?>
+--EXPECT--
+bool(false)
index c8199c1c14bae0f368a549c5603310e5f39759db..776ad6ba9ef7d360a89957a352a3fa9140fa9de0 100644 (file)
@@ -8549,25 +8549,28 @@ void zend_eval_const_expr(zend_ast **ast_ptr) /* {{{ */
                case ZEND_AST_AND:
                case ZEND_AST_OR:
                {
-                       int i;
-                       for (i = 0; i <= 1; i++) {
-                               zend_eval_const_expr(&ast->child[i]);
-                               if (ast->child[i]->kind == ZEND_AST_ZVAL) {
-                                       if (zend_is_true(zend_ast_get_zval(ast->child[i])) == (ast->kind == ZEND_AST_OR)) {
-                                               ZVAL_BOOL(&result, ast->kind == ZEND_AST_OR);
-                                               return;
-                                       }
-                               }
+                       zend_bool child0_is_true, child1_is_true;
+                       zend_eval_const_expr(&ast->child[0]);
+                       zend_eval_const_expr(&ast->child[1]);
+                       if (ast->child[0]->kind != ZEND_AST_ZVAL) {
+                               return;
                        }
 
-                       if (ast->child[0]->kind != ZEND_AST_ZVAL || ast->child[1]->kind != ZEND_AST_ZVAL) {
+                       child0_is_true = zend_is_true(zend_ast_get_zval(ast->child[0]));
+                       if (child0_is_true == (ast->kind == ZEND_AST_OR)) {
+                               ZVAL_BOOL(&result, ast->kind == ZEND_AST_OR);
+                               break;
+                       }
+
+                       if (ast->child[1]->kind != ZEND_AST_ZVAL) {
                                return;
                        }
 
+                       child1_is_true = zend_is_true(zend_ast_get_zval(ast->child[1]));
                        if (ast->kind == ZEND_AST_OR) {
-                               ZVAL_BOOL(&result, zend_is_true(zend_ast_get_zval(ast->child[0])) || zend_is_true(zend_ast_get_zval(ast->child[1])));
+                               ZVAL_BOOL(&result, child0_is_true || child1_is_true);
                        } else {
-                               ZVAL_BOOL(&result, zend_is_true(zend_ast_get_zval(ast->child[0])) && zend_is_true(zend_ast_get_zval(ast->child[1])));
+                               ZVAL_BOOL(&result, child0_is_true && child1_is_true);
                        }
                        break;
                }