ext/standard: Validate objects before queuing unserialize callbacks

Check the closing brace before queuing __unserialize() or __wakeup().
Incomplete object records must use the same cleanup as invalid property
data, including destructor suppression. The __unserialize() data array of
a failed record now stays in the unserialize context until var_destroy()
instead of being released at once, so a destructor of a nested object
cannot resolve a back-reference to an element the array already released.

Author: iliaal

NEWS

@@ -96,6 +96,8 @@ PHP                                                                        NEWS
     file fails after it was already truncated. (Ilia Alshanetsky)
 
 - Standard:
+  . Fixed __unserialize() and __wakeup() being called for objects with a missing
+    closing brace. (Ilia Alshanetsky)
   . Fixed chown() and lchown() failing to resolve user names in ZTS builds
     when getpwnam_r() needs a larger buffer. (Ilia Alshanetsky)
   . Fixed password_get_info() and password_needs_rehash() accepting malformed

ext/date/tests/unserialize-test.phpt

@@ -31,7 +31,6 @@ bool(false)
 
 ossfuzz-55599.txt: 
 %s: unserialize(): Error at offset 26 of 26 bytes in %sunserialize-test.php on line %d
-Error: Invalid serialization data for DateTime object
 bool(false)
 
 
@@ -42,6 +41,4 @@ bool(false)
 
 ossfuzz-56931.txt: 
 %s: unserialize(): Error at offset 39 of 39 bytes in %sunserialize-test.php on line %d
-
-Deprecated: Creation of dynamic property DateInterval::$ is deprecated in %s on line %d
 bool(false)

ext/standard/tests/serialize/__unserialize_malformed.phpt

@@ -0,0 +1,38 @@
+--TEST--
+__unserialize() and destructors are not called for incomplete objects
+--FILE--
+<?php
+class Test {
+    public function __unserialize(array $data): void {
+        echo "__unserialize: ";
+        var_dump($data);
+    }
+
+    public function __destruct() {
+        echo "__destruct\n";
+    }
+}
+
+foreach ([
+    'empty' => 'O:4:"Test":0:{',
+    'nonempty' => 'O:4:"Test":1:{i:0;i:42;',
+    'nested' => 'a:1:{i:0;O:4:"Test":0:{',
+] as $name => $data) {
+    echo "$name: ";
+    var_dump(@unserialize($data));
+}
+
+echo "valid: ";
+var_dump(unserialize('O:4:"Test":1:{i:0;i:42;}'));
+?>
+--EXPECT--
+empty: bool(false)
+nonempty: bool(false)
+nested: bool(false)
+valid: __unserialize: array(1) {
+  [0]=>
+  int(42)
+}
+object(Test)#1 (0) {
+}
+__destruct

ext/standard/tests/serialize/__unserialize_malformed_nested_destructor.phpt

@@ -0,0 +1,27 @@
+--TEST--
+Destructor of a nested object in a failed __unserialize() payload cannot reach freed slots
+--FILE--
+<?php
+class T {
+    public function __unserialize(array $data): void {}
+}
+
+class D {
+    public function __destruct() {
+        var_dump(unserialize('R:2;'));
+    }
+}
+
+foreach ([
+    'missing brace' => 'O:1:"T":2:{i:0;s:5:"hello";i:1;O:1:"D":0:{}',
+    'malformed element' => 'O:1:"T":3:{i:0;s:5:"hello";i:1;O:1:"D":0:{}i:2;X',
+] as $name => $data) {
+    echo "$name: ";
+    var_dump(@unserialize($data));
+}
+?>
+--EXPECT--
+missing brace: bool(false)
+bool(false)
+malformed element: bool(false)
+bool(false)

ext/standard/tests/serialize/__wakeup_malformed.phpt

@@ -0,0 +1,38 @@
+--TEST--
+__wakeup() and destructors are not called for incomplete objects
+--FILE--
+<?php
+class Test {
+    public int $value = 0;
+
+    public function __wakeup(): void {
+        echo "__wakeup: $this->value\n";
+    }
+
+    public function __destruct() {
+        echo "__destruct\n";
+    }
+}
+
+foreach ([
+    'empty' => 'O:4:"Test":0:{',
+    'nonempty' => 'O:4:"Test":1:{s:5:"value";i:42;',
+    'nested' => 'a:1:{i:0;O:4:"Test":0:{',
+] as $name => $data) {
+    echo "$name: ";
+    var_dump(@unserialize($data));
+}
+
+echo "valid: ";
+var_dump(unserialize('O:4:"Test":1:{s:5:"value";i:42;}'));
+?>
+--EXPECT--
+empty: bool(false)
+nonempty: bool(false)
+nested: bool(false)
+valid: __wakeup: 42
+object(Test)#1 (1) {
+  ["value"]=>
+  int(42)
+}
+__destruct

ext/standard/var_unserializer.re

@@ -799,10 +799,11 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
 		array_init_size(&ary, elements);
 		/* Avoid reallocation due to packed -> mixed conversion. */
 		zend_hash_real_init_mixed(Z_ARRVAL(ary));
-		if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)) {
+		if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)
+				|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
 			ZVAL_DEREF(rval);
 			GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
-			zval_ptr_dtor(&ary);
+			ZVAL_COPY_VALUE(var_tmp_var(var_hash), &ary);
 			return 0;
 		}
 
@@ -815,7 +816,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
 		tmp++;
 		ZVAL_COPY_VALUE(tmp, &ary);
 
-		return finish_nested_data(UNSERIALIZE_PASSTHRU);
+		return 1;
 	}
 
 	has_wakeup = Z_OBJCE_P(rval) != PHP_IC_ENTRY
@@ -827,7 +828,8 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
 	}
 
 	zend_hash_extend(ht, zend_hash_num_elements(ht) + elements, HT_IS_PACKED(ht));
-	if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))) {
+	if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))
+			|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
 		if (has_wakeup) {
 			ZVAL_DEREF(rval);
 			GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
@@ -843,7 +845,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
 		Z_EXTRA_P(wakeup_var) = VAR_WAKEUP_FLAG;
 	}
 
-	return finish_nested_data(UNSERIALIZE_PASSTHRU);
+	return 1;
 }
 #ifdef PHP_WIN32
 # pragma optimize("", on)