Respect typed references in catch assignment

nikic · nikic · commit 4a08ca1294ea · 2020-05-26T14:59:40.000+02:00
I decided to null out EG(exception) early here, which means only
the exception from the dtor / ref assign is preserved, and the
previous exception is not chained in. This is more robust, and
I don't think this situation is common enough to be bothered about
the precise behavior.
diff --git a/Zend/tests/bug53511.phpt b/Zend/tests/bug53511.phpt
@@ -20,12 +20,7 @@ function test() {
 test();
 echo "bug\n";
 --EXPECTF--
-Fatal error: Uncaught Exception: ops 2 in %sbug53511.php:11
-Stack trace:
-#0 %sbug53511.php(17): test()
-#1 {main}
-
-Next Exception: ops 1 in %sbug53511.php:4
+Fatal error: Uncaught Exception: ops 1 in %sbug53511.php:4
 Stack trace:
 #0 %sbug53511.php(12): Foo->__destruct()
 #1 %sbug53511.php(17): test()
diff --git a/Zend/tests/type_declarations/typed_properties_108.phpt b/Zend/tests/type_declarations/typed_properties_108.phpt
@@ -0,0 +1,47 @@
+--TEST--
+Variable assignment in catch must respect typed references
+--FILE--
+<?php
+
+class Test {
+    public int $i = 42;
+    public string $s = "str";
+}
+
+$test = new Test;
+
+$ref =& $test->i;
+try {
+    try {
+        throw new Exception("ex");
+    } catch (Exception $ref) {
+        echo "Unreachable\n";
+    }
+} catch (TypeError $e) {
+    var_dump($test->i);
+    echo $e . "\n\n";
+}
+
+$ref =& $test->s;
+try {
+    try {
+        throw new Exception("ex");
+    } catch (Exception $ref) {
+        echo "Unreachable\n";
+    }
+} catch (TypeError $e) {
+    var_dump($test->s);
+    echo $e . "\n\n";
+}
+
+?>
+--EXPECTF--
+int(42)
+TypeError: Cannot assign Exception to reference held by property Test::$i of type int in %s:%d
+Stack trace:
+#0 {main}
+
+string(3) "str"
+TypeError: Cannot assign Exception to reference held by property Test::$s of type string in %s:%d
+Stack trace:
+#0 {main}
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
@@ -4632,17 +4632,15 @@ ZEND_VM_HANDLER(107, ZEND_CATCH, CONST, JMP_ADDR, LAST_CATCH|CACHE_SLOT)
 
 	exception = EG(exception);
 	ex = EX_VAR(opline->result.var);
-	if (UNEXPECTED(Z_ISREF_P(ex))) {
-		ex = Z_REFVAL_P(ex);
-	}
-	zval_ptr_dtor(ex);
-	ZVAL_OBJ(ex, EG(exception));
-	if (UNEXPECTED(EG(exception) != exception)) {
-		GC_ADDREF(EG(exception));
-		HANDLE_EXCEPTION();
-	} else {
+	{
+		/* Always perform a strict assignment. There is a reasonable expectation that if you
+		 * write "catch (Exception $e)" then $e will actually be instanceof Exception. As such,
+		 * we should not permit coercion to string here. */
+		zval tmp;
+		ZVAL_OBJ(&tmp, exception);
 		EG(exception) = NULL;
-		ZEND_VM_NEXT_OPCODE();
+		zend_assign_to_variable(ex, &tmp, IS_TMP_VAR, /* strict */ 1);
+		ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
 	}
 }
 
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
@@ -3753,17 +3753,15 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CATCH_SPEC_CONST_HANDLER(ZEND_
 
 	exception = EG(exception);
 	ex = EX_VAR(opline->result.var);
-	if (UNEXPECTED(Z_ISREF_P(ex))) {
-		ex = Z_REFVAL_P(ex);
-	}
-	zval_ptr_dtor(ex);
-	ZVAL_OBJ(ex, EG(exception));
-	if (UNEXPECTED(EG(exception) != exception)) {
-		GC_ADDREF(EG(exception));
-		HANDLE_EXCEPTION();
-	} else {
+	{
+		/* Always perform a strict assignment. There is a reasonable expectation that if you
+		 * write "catch (Exception $e)" then $e will actually be instanceof Exception. As such,
+		 * we should not permit coercion to string here. */
+		zval tmp;
+		ZVAL_OBJ(&tmp, exception);
 		EG(exception) = NULL;
-		ZEND_VM_NEXT_OPCODE();
+		zend_assign_to_variable(ex, &tmp, IS_TMP_VAR, /* strict */ 1);
+		ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
 	}
 }
 
