Do not allow side-effects when readonly property modification fails (#10757)

Author: kocsismate

Zend/tests/readonly_props/readonly_assign_no_sideeffect.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Test that there can be no side-effect when a readonly property modification fails
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly string $bar
+    ) {}
+
+    public function write() {
+        $this->bar = new S();
+    }
+}
+
+class S {
+    public function __toString() {
+        var_dump("Side-effect in __toString()");
+        return "";
+    }
+}
+
+$foo = new Foo("");
+
+try {
+    $foo->write();
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+Cannot modify readonly property Foo::$bar

Zend/zend_execute.c

@@ -1009,6 +1009,11 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf
 {
 	zval tmp;
 
+	if (UNEXPECTED((info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE))) {
+		zend_readonly_property_modification_error(info);
+		return &EG(uninitialized_zval);
+	}
+
 	ZVAL_DEREF(value);
 	ZVAL_COPY(&tmp, value);
 
@@ -1017,15 +1022,7 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf
 		return &EG(uninitialized_zval);
 	}
 
-	if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) {
-		if (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE) {
-			Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
-		} else {
-			zval_ptr_dtor(&tmp);
-			zend_readonly_property_modification_error(info);
-			return &EG(uninitialized_zval);
-		}
-	}
+	Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
 
 	return zend_assign_to_variable(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES());
 }

Zend/zend_object_handlers.c

@@ -812,6 +812,13 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 			Z_TRY_ADDREF_P(value);
 
 			if (UNEXPECTED(prop_info)) {
+				if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE))) {
+					Z_TRY_DELREF_P(value);
+					zend_readonly_property_modification_error(prop_info);
+					variable_ptr = &EG(error_zval);
+					goto exit;
+				}
+
 				ZVAL_COPY_VALUE(&tmp, value);
 				// Increase refcount to prevent object from being released in __toString()
 				GC_ADDREF(zobj);
@@ -828,16 +835,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 					variable_ptr = &EG(error_zval);
 					goto exit;
 				}
-				if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
-					if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE) {
-						Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE;
-					} else {
-						zval_ptr_dtor(&tmp);
-						zend_readonly_property_modification_error(prop_info);
-						variable_ptr = &EG(error_zval);
-						goto exit;
-					}
-				}
+				Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE;
 				value = &tmp;
 			}