Fixed bug #72347 (VERIFY_RETURN type casts visible in finally)

Fixed bug #72216 (Return by reference with finally is not memory safe)
Fixed bug #72215 (Wrong return value if var modified in finally)

diff --git a/NEWS b/NEWS
index 1d885619dee2d1a91517a81a51a3a0d94a87215b..91bba83d05251606e52e78f44406a4ad45f5bdf5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,10 @@ PHP                                                                        NEWS
     deserialization). (Laruence)
   . Fixed bug #72543 (Different references behavior comparing to PHP 5)
     (Laruence, Dmitry, Nikita)
+  . Fixed bug #72347 (VERIFY_RETURN type casts visible in finally). (Dmitry)
+  . Fixed bug #72216 (Return by reference with finally is not memory safe).
+    (Dmitry)
+  . Fixed bug #72215 (Wrong return value if var modified in finally). (Dmitry)
   . Fixed bug #71539 (Memory error on $arr[$a] =& $arr[$b] if RHS rehashes)
     (Dmitry, Nikita)
   . Added new constant PHP_FD_SETSIZE. (cmb)

diff --git a/Zend/tests/bug72215.phpt b/Zend/tests/bug72215.phpt
new file mode 100644 (file)
index 0000000..0ff1629
--- /dev/null
+++ b/Zend/tests/bug72215.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Bug #72215 (Wrong return value if var modified in finally)
+--FILE--
+<?php
+function test() {
+    $a = 1;
+    try {
+        return $a;
+    } finally {
+        $a = 2;
+    }
+}
+var_dump(test());
+?>
+--EXPECT--
+int(1)

diff --git a/Zend/tests/bug72215_1.phpt b/Zend/tests/bug72215_1.phpt
new file mode 100644 (file)
index 0000000..d56c9f8
--- /dev/null
+++ b/Zend/tests/bug72215_1.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #72215.1 (Wrong return value if var modified in finally)
+--FILE--
+<?php
+function &test(&$b) {
+    $a =& $b;
+    try {
+        return $a;
+    } finally {
+        $a = 2;
+    }
+}
+$x = 1;
+$y =& test($x);
+var_dump($y);
+$x = 3;
+var_dump($y);
+?>
+--EXPECT--
+int(2)
+int(3)

diff --git a/Zend/tests/bug72215_2.phpt b/Zend/tests/bug72215_2.phpt
new file mode 100644 (file)
index 0000000..cefb6d9
--- /dev/null
+++ b/Zend/tests/bug72215_2.phpt
@@ -0,0 +1,22 @@
+--TEST--
+Bug #72215.1 (Wrong return value if var modified in finally)
+--FILE--
+<?php
+function &test(&$b) {
+    $a =& $b;
+    try {
+        return $a;
+    } finally {
+        $a =& $c;
+       $a = 2;
+    }
+}
+$x = 1;
+$y =& test($x);
+var_dump($y);
+$x = 3;
+var_dump($y);
+?>
+--EXPECT--
+int(1)
+int(3)

diff --git a/Zend/tests/bug72216.phpt b/Zend/tests/bug72216.phpt
new file mode 100644 (file)
index 0000000..65b5556
--- /dev/null
+++ b/Zend/tests/bug72216.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Bug #72216 (Return by reference with finally is not memory safe)
+--FILE--
+<?php
+function &test() {
+    $a = ["ok"];
+    try {
+        return $a[0];
+    } finally {
+        $a[""] = 42;
+    }
+}
+var_dump(test());
+?>
+--EXPECT--
+string(2) "ok"

diff --git a/Zend/tests/bug72347.phpt b/Zend/tests/bug72347.phpt
new file mode 100644 (file)
index 0000000..b864572
--- /dev/null
+++ b/Zend/tests/bug72347.phpt
@@ -0,0 +1,17 @@
+--TEST--
+Bug #72347 (VERIFY_RETURN type casts visible in finally)
+--FILE--
+<?php
+function test() : int {
+    $d = 1.5;
+    try {
+        return $d;
+    } finally {
+        var_dump($d);
+    }
+}
+var_dump(test());
+?>
+--EXPECT--
+float(1.5)
+int(1)

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 6c51527e5322b445c7d62d38d6828d6d83b784c5..c6eb087eddc29ac24b0e1ff7f8c0104a6c976cc5 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -4085,6 +4085,38 @@ static int zend_handle_loops_and_finally(znode *return_value) /* {{{ */
 }
 /* }}} */
 
+static int zend_has_finally_ex(zend_long depth) /* {{{ */
+{
+       zend_loop_var *base;
+       zend_loop_var *loop_var = zend_stack_top(&CG(loop_var_stack));
+
+       if (!loop_var) {
+               return 0;
+       }
+       base = zend_stack_base(&CG(loop_var_stack));
+       for (; loop_var >= base; loop_var--) {
+               if (loop_var->opcode == ZEND_FAST_CALL) {
+                       return 1;
+               } else if (loop_var->opcode == ZEND_DISCARD_EXCEPTION) {
+               } else if (loop_var->opcode == ZEND_RETURN) {
+                       /* Stack separator */
+                       return 0;
+               } else if (depth <= 1) {
+                       return 0;
+               } else {
+                       depth--;
+           }
+       }
+       return 0;
+}
+/* }}} */
+
+static int zend_has_finally(void) /* {{{ */
+{
+       return zend_has_finally_ex(zend_stack_count(&CG(loop_var_stack)) + 1);
+}
+/* }}} */
+
 void zend_compile_return(zend_ast *ast) /* {{{ */
 {
        zend_ast *expr_ast = ast->child[0];
@@ -4102,6 +4134,17 @@ void zend_compile_return(zend_ast *ast) /* {{{ */
                zend_compile_expr(&expr_node, expr_ast);
        }
 
+       if ((CG(active_op_array)->fn_flags & ZEND_ACC_HAS_FINALLY_BLOCK)
+        && (expr_node.op_type == IS_CV || (by_ref && expr_node.op_type == IS_VAR))
+        && zend_has_finally()) {
+               /* Copy return value into temporary VAR to avoid modification in finally code */
+               if (by_ref) {
+                       zend_emit_op(&expr_node, ZEND_MAKE_REF, &expr_node, NULL);
+               } else {
+                       zend_emit_op_tmp(&expr_node, ZEND_QM_ASSIGN, &expr_node, NULL);
+               }
+       }
+
        /* Generator return types are handled separately */
        if (!(CG(active_op_array)->fn_flags & ZEND_ACC_GENERATOR) && CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
                zend_emit_return_type_check(

diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index 5cae574c5366f0debaf4c9985db3c749f3b7d2e5..d10bd72e9d1787988851f9e2e92f166c60df9df0 100644 (file)
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -8102,12 +8102,21 @@ ZEND_VM_HANDLER(49, ZEND_CHECK_VAR, CV, UNUSED)
        ZEND_VM_NEXT_OPCODE();
 }
 
-ZEND_VM_HANDLER(51, ZEND_MAKE_REF, VAR, UNUSED)
+ZEND_VM_HANDLER(51, ZEND_MAKE_REF, VAR|CV, UNUSED)
 {
        USE_OPLINE
        zval *op1 = EX_VAR(opline->op1.var);
 
-       if (EXPECTED(Z_TYPE_P(op1) == IS_INDIRECT)) {
+       if (OP1_TYPE == IS_CV) {
+               if (UNEXPECTED(Z_TYPE_P(op1) == IS_UNDEF)) {
+                       SAVE_OPLINE();
+                       GET_OP1_UNDEF_CV(op1, BP_VAR_R);
+                       ZVAL_NULL(EX_VAR(opline->result.var));
+                       ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
+               }
+               ZVAL_MAKE_REF(op1);
+               ZVAL_COPY(EX_VAR(opline->result.var), op1);
+       } else if (EXPECTED(Z_TYPE_P(op1) == IS_INDIRECT)) {
                op1 = Z_INDIRECT_P(op1);
                if (EXPECTED(!Z_ISREF_P(op1))) {
                        ZVAL_MAKE_REF(op1);

diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index 83eb35f610c315cd377fd1077f660d03ece78d61..37d8862d3b9b3a551599b670c6bcc640823a9134 100644 (file)
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -21826,7 +21826,16 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_MAKE_REF_SPEC_VAR_UNUSED_HANDL
        USE_OPLINE
        zval *op1 = EX_VAR(opline->op1.var);
 
-       if (EXPECTED(Z_TYPE_P(op1) == IS_INDIRECT)) {
+       if (IS_VAR == IS_CV) {
+               if (UNEXPECTED(Z_TYPE_P(op1) == IS_UNDEF)) {
+                       SAVE_OPLINE();
+                       GET_OP1_UNDEF_CV(op1, BP_VAR_R);
+                       ZVAL_NULL(EX_VAR(opline->result.var));
+                       ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
+               }
+               ZVAL_MAKE_REF(op1);
+               ZVAL_COPY(EX_VAR(opline->result.var), op1);
+       } else if (EXPECTED(Z_TYPE_P(op1) == IS_INDIRECT)) {
                op1 = Z_INDIRECT_P(op1);
                if (EXPECTED(!Z_ISREF_P(op1))) {
                        ZVAL_MAKE_REF(op1);
@@ -42809,6 +42818,33 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CHECK_VAR_SPEC_CV_UNUSED_HANDL
        ZEND_VM_NEXT_OPCODE();
 }
 
+static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_MAKE_REF_SPEC_CV_UNUSED_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
+{
+       USE_OPLINE
+       zval *op1 = EX_VAR(opline->op1.var);
+
+       if (IS_CV == IS_CV) {
+               if (UNEXPECTED(Z_TYPE_P(op1) == IS_UNDEF)) {
+                       SAVE_OPLINE();
+                       GET_OP1_UNDEF_CV(op1, BP_VAR_R);
+                       ZVAL_NULL(EX_VAR(opline->result.var));
+                       ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
+               }
+               ZVAL_MAKE_REF(op1);
+               ZVAL_COPY(EX_VAR(opline->result.var), op1);
+       } else if (EXPECTED(Z_TYPE_P(op1) == IS_INDIRECT)) {
+               op1 = Z_INDIRECT_P(op1);
+               if (EXPECTED(!Z_ISREF_P(op1))) {
+                       ZVAL_MAKE_REF(op1);
+               }
+               GC_REFCOUNT(Z_REF_P(op1))++;
+               ZVAL_REF(EX_VAR(opline->result.var), Z_REF_P(op1));
+       } else {
+               ZVAL_COPY_VALUE(EX_VAR(opline->result.var), op1);
+       }
+       ZEND_VM_NEXT_OPCODE();
+}
+
 static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ADD_SPEC_CV_CV_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
 {
        USE_OPLINE
@@ -58506,7 +58542,7 @@ void zend_init_opcodes_handlers(void)
                ZEND_NULL_HANDLER,
                ZEND_NULL_HANDLER,
                ZEND_NULL_HANDLER,
-               ZEND_NULL_HANDLER,
+               ZEND_MAKE_REF_SPEC_CV_UNUSED_HANDLER,
                ZEND_NULL_HANDLER,
                ZEND_BOOL_SPEC_CONST_HANDLER,
                ZEND_BOOL_SPEC_TMPVAR_HANDLER,