Allow readonly properties to be reset once during cloning

kocsismate · kocsismate · commit dd465166c43b · 2022-09-07T10:08:05.000+02:00
diff --git a/Zend/tests/readonly_props/readonly_clone_error.phpt b/Zend/tests/readonly_props/readonly_clone_error.phpt
@@ -0,0 +1,33 @@
+--TEST--
+Readonly property cannot be reset twice during cloning
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar
+    ) {}
+
+    public function __clone()
+    {
+        $this->bar++;
+        var_dump($this);
+        $this->bar++;
+    }
+}
+
+$foo = new Foo(1);
+
+try {
+    clone $foo;
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+object(Foo)#2 (1) {
+  ["bar"]=>
+  int(2)
+}
+Cannot modify readonly property Foo::$bar
diff --git a/Zend/tests/readonly_props/readonly_clone_success.phpt b/Zend/tests/readonly_props/readonly_clone_success.phpt
@@ -0,0 +1,51 @@
+--TEST--
+Readonly property can be reset once during cloning
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar,
+        public readonly int $baz
+    ) {}
+
+    public function __clone()
+    {
+        $this->bar++;
+    }
+
+    public function wrongClone()
+    {
+        $instance = clone $this;
+        $instance->baz++;
+    }
+}
+
+$foo = new Foo(1, 1);
+$foo2 = clone $foo;
+var_dump($foo2);
+
+try {
+    $foo->wrongClone();
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+$foo3 = clone $foo2;
+var_dump($foo2);
+
+?>
+--EXPECT--
+object(Foo)#2 (2) {
+  ["bar"]=>
+  int(2)
+  ["baz"]=>
+  int(0)
+}
+Cannot modify readonly property Foo::$baz
+object(Foo)#2 (2) {
+  ["bar"]=>
+  int(2)
+  ["baz"]=>
+  int(0)
+}
diff --git a/Zend/zend_API.c b/Zend/zend_API.c
@@ -1693,6 +1693,7 @@ static zend_always_inline zend_result _object_and_properties_init(zval *arg, zen
 
 	if (class_type->create_object == NULL) {
 		zend_object *obj = zend_objects_new(class_type);
+		obj->in_clone = 0;
 
 		ZVAL_OBJ(arg, obj);
 		if (properties) {
@@ -1703,6 +1704,7 @@ static zend_always_inline zend_result _object_and_properties_init(zval *arg, zen
 	} else {
 		ZVAL_OBJ(arg, class_type->create_object(class_type));
 	}
+
 	return SUCCESS;
 }
 /* }}} */
diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
@@ -634,7 +634,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
 				}
 			}
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT)) {
 			/* Skip __get() for uninitialized typed properties */
 			goto uninit_error;
 		}
@@ -812,10 +812,14 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 
 			if (UNEXPECTED(prop_info)) {
 				if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
-					Z_TRY_DELREF_P(value);
-					zend_readonly_property_modification_error(prop_info);
-					variable_ptr = &EG(error_zval);
-					goto exit;
+					if (zobj->in_clone && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINIT)) {
+						Z_PROP_FLAG_P(variable_ptr) |= IS_PROP_REINIT;
+					} else {
+						Z_TRY_DELREF_P(value);
+						zend_readonly_property_modification_error(prop_info);
+						variable_ptr = &EG(error_zval);
+						goto exit;
+					}
 				}
 
 				ZVAL_COPY_VALUE(&tmp, value);
@@ -832,7 +836,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 				variable_ptr, value, IS_TMP_VAR, property_uses_strict_types());
 			goto exit;
 		}
-		if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) {
+		if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_UNINIT) {
 			/* Writes to uninitialized typed properties bypass __set(). */
 			goto write_std_property;
 		}
@@ -1050,7 +1054,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
 		if (UNEXPECTED(Z_TYPE_P(retval) == IS_UNDEF)) {
 			if (EXPECTED(!zobj->ce->__get) ||
 			    UNEXPECTED((*zend_get_property_guard(zobj, name)) & IN_GET) ||
-			    UNEXPECTED(prop_info && Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) {
+			    UNEXPECTED(prop_info && (Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT))) {
 				if (UNEXPECTED(type == BP_VAR_RW || type == BP_VAR_R)) {
 					if (UNEXPECTED(prop_info)) {
 						zend_throw_error(NULL,
@@ -1145,7 +1149,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
 			}
 			return;
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(slot) & IS_PROP_UNINIT)) {
 			if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY)
 					&& !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) {
 				return;
@@ -1760,7 +1764,7 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
 		if (Z_TYPE_P(value) != IS_UNDEF) {
 			goto found;
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(value) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(value) & IS_PROP_UNINIT)) {
 			/* Skip __isset() for uninitialized typed properties */
 			result = 0;
 			goto exit;
diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
@@ -33,6 +33,7 @@ static zend_always_inline void _zend_object_std_init(zend_object *object, zend_c
 	object->ce = ce;
 	object->handlers = ce->default_object_handlers;
 	object->properties = NULL;
+	object->in_clone = 0;
 	zend_objects_store_put(object);
 	if (UNEXPECTED(ce->ce_flags & ZEND_ACC_USE_GUARDS)) {
 		ZVAL_UNDEF(object->properties_table + object->ce->default_properties_count);
@@ -198,6 +199,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 		zval *end = src + old_object->ce->default_properties_count;
 
 		do {
+			Z_PROP_FLAG_P(src) &= ~IS_PROP_REINIT;
 			i_zval_ptr_dtor(dst);
 			ZVAL_COPY_VALUE_PROP(dst, src);
 			zval_add_ref(dst);
@@ -255,7 +257,9 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 
 	if (old_object->ce->clone) {
 		GC_ADDREF(new_object);
+		new_object->in_clone = 1;
 		zend_call_known_instance_method_with_0_params(new_object->ce->clone, new_object, NULL);
+		new_object->in_clone = 0;
 		OBJ_RELEASE(new_object);
 	}
 }
diff --git a/Zend/zend_types.h b/Zend/zend_types.h
@@ -503,6 +503,7 @@ struct _zend_object {
 	const zend_object_handlers *handlers;
 	HashTable        *properties;
 	zval              properties_table[1];
+	bool              in_clone;
 };
 
 struct _zend_resource {
@@ -1438,13 +1439,14 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
  * the Z_EXTRA space when copying property default values etc. We define separate
  * macros for this purpose, so this workaround is easier to remove in the future. */
 #define IS_PROP_UNINIT 1
+#define IS_PROP_REINIT 2
 #define Z_PROP_FLAG_P(z) Z_EXTRA_P(z)
 #define ZVAL_COPY_VALUE_PROP(z, v) \
 	do { *(z) = *(v); } while (0)
 #define ZVAL_COPY_PROP(z, v) \
-	do { ZVAL_COPY(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+	do { ZVAL_COPY(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v) & IS_PROP_UNINIT; } while (0)
 #define ZVAL_COPY_OR_DUP_PROP(z, v) \
-	do { ZVAL_COPY_OR_DUP(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+	do { ZVAL_COPY_OR_DUP(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v) & IS_PROP_UNINIT; } while (0)
 
 
 #endif /* ZEND_TYPES_H */

Original file line number	Diff line number	Diff line change
`@@ -1693,6 +1693,7 @@ static zend_always_inline zend_result _object_and_properties_init(zval *arg, zen`
`1693`	`1693`
`1694`	`1694`	`if (class_type->create_object == NULL) {`
`1695`	`1695`	`zend_object *obj = zend_objects_new(class_type);`
	`1696`	`+ obj->in_clone = 0;`
`1696`	`1697`
`1697`	`1698`	`ZVAL_OBJ(arg, obj);`
`1698`	`1699`	`if (properties) {`
`@@ -1703,6 +1704,7 @@ static zend_always_inline zend_result _object_and_properties_init(zval *arg, zen`
`1703`	`1704`	`} else {`
`1704`	`1705`	`ZVAL_OBJ(arg, class_type->create_object(class_type));`
`1705`	`1706`	`}`
	`1707`	`+`
`1706`	`1708`	`return SUCCESS;`
`1707`	`1709`	`}`
`1708`	`1710`	`/* }}} */`