zend_API: Verify property types in object_properties_load()

NEWS

@@ -5,6 +5,8 @@ PHP                                                                        NEWS
 - Core:
   . Fixed incorrect internal pointer and foreach iterator positions when
     compacting arrays with holes. (Weilin Du)
+  . Fix handling of references to typed properties during unserialization
+    of various internal classes. (ndossche, timwolla)
 
 - DOM:
   . Fixed use-after-free when re-constructing a DOMXPath whose php:function

UPGRADING.INTERNALS

@@ -182,6 +182,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES
     instead of a zval*. Accordingly, zend_get_closure_this_ptr() now returns
     that zend_object*, or NULL when the closure is unbound, instead of a
     zval* that is IS_UNDEF when the closure is unbound.
+  . object_properties_load() now verifies that the given value is assignable
+    to typed properties. The check is performed in strict mode.
 
 - Added:
   . New zend_class_entry.ce_flags2 and zend_function.fn_flags2 fields were

Zend/zend_API.c

@@ -1758,7 +1758,7 @@ ZEND_API void object_properties_load(zend_object *object, const HashTable *prope
 	zval *prop, tmp;
 	zend_string *key;
 	zend_long h;
-	const zend_property_info *property_info;
+	zend_property_info *property_info;
 
 	ZEND_HASH_FOREACH_KEY_VAL(properties, h, key, prop) {
 		if (key) {
@@ -1785,18 +1785,56 @@ ZEND_API void object_properties_load(zend_object *object, const HashTable *prope
 			if (property_info != ZEND_WRONG_PROPERTY_INFO &&
 				property_info &&
 				(property_info->flags & ZEND_ACC_STATIC) == 0) {
+				bool is_typed = ZEND_TYPE_IS_SET(property_info->type);
+
+				/* Mimick unserialize behaviour for virtual properties. */
+				if (UNEXPECTED(property_info->flags & ZEND_ACC_VIRTUAL)) {
+					zend_throw_error(NULL, "Cannot unserialize value for virtual property %s::$%s", ZSTR_VAL(object->ce->name), zend_get_unmangled_property_name(property_info->name));
+					return;
+				}
+
 				zval *slot = OBJ_PROP(object, property_info->offset);
-				if (UNEXPECTED((property_info->flags & ZEND_ACC_READONLY) && !Z_ISUNDEF_P(slot))) {
-					if (Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE) {
-						Z_PROP_FLAG_P(slot) &= ~IS_PROP_REINITABLE;
+				zval val;
+
+				if (is_typed) {
+					if (UNEXPECTED(Z_ISREF_P(prop))) {
+						/* Block taking a reference to a readonly property. */
+						if (UNEXPECTED(property_info->flags & ZEND_ACC_READONLY)) {
+							zend_readonly_property_indirect_modification_error(property_info);
+							return;
+						}
+						if (UNEXPECTED(!zend_verify_prop_assignable_by_ref(property_info, prop, /* strict */ true))) {
+							ZEND_ASSERT(EG(exception));
+							return;
+						}
+						ZVAL_COPY(&val, prop);
+						ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(&val), property_info);
 					} else {
-						zend_readonly_property_modification_error(property_info);
-						return;
+						/* Mimick zend_assign_to_typed_prop() by reporting the error before doing work. */
+						if (UNEXPECTED((property_info->flags & ZEND_ACC_READONLY)
+						 && !Z_ISUNDEF_P(slot)
+						 && !(Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE))) {
+							zend_readonly_property_modification_error(property_info);
+							return;
+						}
+
+						ZVAL_COPY(&val, prop);
+						if (UNEXPECTED(!zend_verify_property_type(property_info, &val, /* strict */ true))) {
+							zval_ptr_dtor(&val);
+							return;
+						}
 					}
+					if (UNEXPECTED(Z_ISREF_P(slot))
+					 && (ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(slot)))) {
+						ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(slot), property_info);
+					}
+				} else {
+					ZVAL_COPY(&val, prop);
 				}
+
+				Z_PROP_FLAG_P(slot) &= ~IS_PROP_REINITABLE;
 				zval_ptr_dtor(slot);
-				ZVAL_COPY_VALUE(slot, prop);
-				zval_add_ref(slot);
+				ZVAL_COPY_VALUE(slot, &val);
 				if (object->properties) {
 					ZVAL_INDIRECT(&tmp, slot);
 					zend_hash_update(object->properties, key, &tmp);

ext/date/tests/time/duration/gh23639.phpt

@@ -0,0 +1,34 @@
+--TEST--
+GH-23639 (object_properties_load allows creating readonly reference properties)
+--CREDITS--
+arnaud-lb
+ndossche
+--XFAIL--
+Test can only succeed when GH-23629 is also merged
+--FILE--
+<?php
+
+class Time_Duration {
+    public int $seconds;
+    public int $nanoseconds;
+    public bool $negative;
+}
+
+$d = new Time_Duration();
+$d->seconds = 1;
+$a = [$d, &$d->seconds];
+
+$payload = serialize($a);
+
+try {
+    unserialize(str_replace('Time_Duration', 'Time\\Duration', $payload));
+} catch (Throwable $e) {
+    do {
+        echo $e::class, ": ", $e->getMessage(), "\n";
+    } while ($e = $e->getPrevious());
+}
+
+?>
+--EXPECT--
+Exception: Invalid serialization data for Time\Duration object
+Error: Cannot indirectly modify readonly property Time\Duration::$seconds

ext/random/randomizer.c

@@ -508,7 +508,6 @@ PHP_METHOD(Random_Randomizer, __unserialize)
 	php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS);
 	HashTable *d;
 	zval *members_zv;
-	zval *zengine;
 
 	ZEND_PARSE_PARAMETERS_START(1, 1)
 		Z_PARAM_ARRAY_HT(d);
@@ -531,12 +530,7 @@ PHP_METHOD(Random_Randomizer, __unserialize)
 		RETURN_THROWS();
 	}
 
-	zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", strlen("engine"), 1, NULL);
-	if (Z_TYPE_P(zengine) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(zengine), random_ce_Random_Engine)) {
-		zend_throw_exception(NULL, "Invalid serialization data for Random\\Randomizer object", 0);
-		RETURN_THROWS();
-	}
-
+	zval *zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", strlen("engine"), /* silent */ true, NULL);
 	randomizer_common_init(randomizer, Z_OBJ_P(zengine));
 }
 /* }}} */

ext/random/tests/03_randomizer/gh_9708_unserialize.phpt

@@ -0,0 +1,14 @@
+--TEST--
+GH-9708: object_properties_load() bypasses typed property checks
+--FILE--
+<?php
+
+try {
+    unserialize('O:17:"Random\Randomizer":1:{i:0;a:1:{s:6:"engine";N;}}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Exception: Invalid serialization data for Random\Randomizer object

ext/spl/tests/ArrayObject/gh_9707_unserialize.phpt

@@ -0,0 +1,41 @@
+--TEST--
+GH-9707: object_properties_load crashes in debug mode when unserializing references to typed properties in php 8.1+
+--FILE--
+<?php
+
+class Foo extends ArrayObject {
+    public int $a = 0;
+    public int $b = 0;
+}
+
+$f = new Foo();
+$r = &$f->a;
+$f->b = &$r;
+
+$f->b = 1;
+var_dump($unserialized = unserialize(serialize($f)));
+
+$unserialized->b = 2;
+
+var_dump($unserialized);
+
+?>
+--EXPECTF--
+object(Foo)#%d (3) {
+  ["a"]=>
+  &int(1)
+  ["b"]=>
+  &int(1)
+  ["storage":"ArrayObject":private]=>
+  array(0) {
+  }
+}
+object(Foo)#%d (3) {
+  ["a"]=>
+  &int(2)
+  ["b"]=>
+  &int(2)
+  ["storage":"ArrayObject":private]=>
+  array(0) {
+  }
+}

ext/spl/tests/ArrayObject/gh_9708_unserialize.phpt

@@ -0,0 +1,21 @@
+--TEST--
+GH-9708: object_properties_load() bypasses typed property checks
+--FILE--
+<?php
+
+class Foo extends ArrayObject {
+    public int $a = 5;
+    public string $b = "10";
+}
+
+try {
+    // a = "10", b = 5
+    unserialize('O:3:"Foo":4:{i:0;i:0;i:1;a:0:{}i:2;a:2:{s:1:"a";s:2:"10";s:1:"b";i:5;}i:3;N;}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+
+?>
+--EXPECT--
+TypeError: Cannot assign string to property Foo::$a of type int