Do not pass a CV slot to zend_assign_to_typed_ref_ex() with value_type=IS_TMP_VAR

zend_assign_to_typed_ref_ex() receives a zval slot and tries to dtor it when
value_type=IS_TMP_VAR. This slot may be modified before dtor if it was in fact a
CV slot. In particular it may be turned into a ref, replaced by anyother ref, or
unset; leading to UAFs or leaks.

Fix by passing a stack slot instead. This is consistent with
zend_assign_to_typed_prop().

Author: arnaud-lb

Zend/tests/gh20316-001.phpt

@@ -0,0 +1,37 @@
+--TEST--
+GH-20316 001: Assign to ref: non-ref rvalue may be turned into a ref
+--FILE--
+<?php
+
+class C {
+    public string $a = '';
+    public $b;
+    function __toString() {
+        global $c; // turns rvalue into a ref
+        return '';
+    }
+}
+
+for ($i = 0; $i < 2; $i++) {
+    $c = new C;
+    $c->b = &$c->a;
+    $c->b = $c;
+
+    var_dump($c);
+    unset($c);
+}
+
+?>
+--EXPECTF--
+object(C)#%d (2) {
+  ["a"]=>
+  &string(0) ""
+  ["b"]=>
+  &string(0) ""
+}
+object(C)#%d (2) {
+  ["a"]=>
+  &string(0) ""
+  ["b"]=>
+  &string(0) ""
+}

Zend/tests/gh20316-002.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-20316 002: Assign to ref: ref rvalue may be unset
+--FILE--
+<?php
+
+class C {
+    public string $a = '';
+    public $b;
+    function __toString() {
+        unset($GLOBALS['c']);
+        unset($GLOBALS['r']);
+        return '';
+    }
+}
+
+for ($i = 0; $i < 2; $i++) {
+    $c = new C;
+    $r = &$c;
+    $c->b = &$c->a;
+    $c->b = $c;
+
+    var_dump(isset($c));
+}
+
+?>
+--EXPECT--
+bool(false)
+bool(false)

Zend/tests/gh20316-003.phpt

@@ -0,0 +1,26 @@
+--TEST--
+GH-20316 003: Assign to ref: rvalue may be unset
+--FILE--
+<?php
+
+class C {
+    public string $a = '';
+    public $b;
+    function __toString() {
+        unset($GLOBALS['c']);
+        return '';
+    }
+}
+
+for ($i = 0; $i < 2; $i++) {
+    $c = new C;
+    $c->b = &$c->a;
+    $c->b = $c;
+
+    var_dump(isset($c));
+}
+
+?>
+--EXPECT--
+bool(false)
+bool(false)

Zend/zend_API.c

@@ -5068,17 +5068,15 @@ ZEND_API zend_result zend_update_static_property_ex(zend_class_entry *scope, zen
 	}
 
 	ZEND_ASSERT(!Z_ISREF_P(value));
-	Z_TRY_ADDREF_P(value);
+	ZVAL_COPY(&tmp, value);
 	if (ZEND_TYPE_IS_SET(prop_info->type)) {
-		ZVAL_COPY_VALUE(&tmp, value);
 		if (!zend_verify_property_type(prop_info, &tmp, /* strict */ 0)) {
-			Z_TRY_DELREF_P(value);
+			zval_ptr_dtor(&tmp);
 			return FAILURE;
 		}
-		value = &tmp;
 	}
 
-	zend_assign_to_variable(property, value, IS_TMP_VAR, /* strict */ 0);
+	zend_assign_to_variable(property, &tmp, IS_TMP_VAR, /* strict */ 0);
 	return SUCCESS;
 }
 /* }}} */

Zend/zend_execute.c

@@ -601,8 +601,9 @@ static zend_never_inline ZEND_COLD zval *zend_wrong_assign_to_variable_reference
 		return &EG(uninitialized_zval);
 	}
 
+	zval tmp;
+	ZVAL_COPY(&tmp, value_ptr);
 	/* Use IS_TMP_VAR instead of IS_VAR to avoid ISREF check */
-	Z_TRY_ADDREF_P(value_ptr);
 	return zend_assign_to_variable_ex(variable_ptr, value_ptr, IS_TMP_VAR, EX_USES_STRICT_TYPES(), garbage_ptr);
 }
 

Zend/zend_object_handlers.c

@@ -1078,11 +1078,10 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 		}
 
 		if (Z_TYPE_P(variable_ptr) != IS_UNDEF) {
-			Z_TRY_ADDREF_P(value);
+			ZVAL_COPY(&tmp, value);
 
 			if (prop_info) {
 typed_property:
-				ZVAL_COPY_VALUE(&tmp, value);
 				// Increase refcount to prevent object from being released in __toString()
 				GC_ADDREF(zobj);
 				bool type_matched = zend_verify_property_type(prop_info, &tmp, property_uses_strict_types());
@@ -1099,14 +1098,13 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 					goto exit;
 				}
 				Z_PROP_FLAG_P(variable_ptr) &= ~(IS_PROP_UNINIT|IS_PROP_REINITABLE);
-				value = &tmp;
 			}
 
 found:;
 			zend_refcounted *garbage = NULL;
 
 			variable_ptr = zend_assign_to_variable_ex(
-				variable_ptr, value, IS_TMP_VAR, property_uses_strict_types(), &garbage);
+				variable_ptr, &tmp, IS_TMP_VAR, property_uses_strict_types(), &garbage);
 
 			if (garbage) {
 				if (GC_DELREF(garbage) == 0) {
@@ -1146,7 +1144,7 @@ found:;
 				zobj->properties = zend_array_dup(zobj->properties);
 			}
 			if ((variable_ptr = zend_hash_find(zobj->properties, name)) != NULL) {
-				Z_TRY_ADDREF_P(value);
+				ZVAL_COPY(&tmp, value);
 				goto found;
 			}
 		}
@@ -1241,12 +1239,12 @@ found:;
 		if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
 			variable_ptr = OBJ_PROP(zobj, property_offset);
 
-			Z_TRY_ADDREF_P(value);
 			if (prop_info) {
+				ZVAL_COPY(&tmp, value);
 				goto typed_property;
 			}
 
-			ZVAL_COPY_VALUE(variable_ptr, value);
+			ZVAL_COPY(variable_ptr, value);
 		} else {
 			if (UNEXPECTED(zobj->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
 				zend_forbidden_dynamic_property(zobj->ce, name);