From: Nikita Popov <nikita.ppv@gmail.com>
Date: Mon, 7 Dec 2020 11:33:23 +0000 (+0100)
Subject: Fix another typed resource issue in unserialization
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2d467abc46ec4ee97484d4e35909bed322600037;p=php

Fix another typed resource issue in unserialization

We also need to discard old entries in the ref_props HT when values
are overwritten.

We should really forbid these kinds of overwrites. I believe they
can only occur in manually crafted serialization strings, and
cause so many problems...

Fixes oss-fuzz #28257.
---

diff --git a/ext/standard/tests/serialize/typed_property_ref_overwrite2.phpt b/ext/standard/tests/serialize/typed_property_ref_overwrite2.phpt
new file mode 100644
index 0000000000..a408bf3196
--- /dev/null
+++ b/ext/standard/tests/serialize/typed_property_ref_overwrite2.phpt
@@ -0,0 +1,22 @@
+--TEST--
+Overwriting a typed property that is not yet a reference
+--FILE--
+<?php
+
+class Test {
+    public ?Test $prop;
+}
+$s = <<<'STR'
+O:4:"Test":2:{s:4:"prop";N;s:4:"prop";O:4:"Test":1:{s:4:"prop";R:2;}}
+STR;
+var_dump(unserialize($s));
+
+?>
+--EXPECT--
+object(Test)#1 (1) {
+  ["prop"]=>
+  &object(Test)#2 (1) {
+    ["prop"]=>
+    *RECURSION*
+  }
+}
diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re
index 1c787034dd..1b191a0367 100644
--- a/ext/standard/var_unserializer.re
+++ b/ext/standard/var_unserializer.re
@@ -556,9 +556,17 @@ string_key:
 						/* This is a property with a declaration */
 						old_data = Z_INDIRECT_P(old_data);
 						info = zend_get_typed_property_info_for_slot(obj, old_data);
-						if (info && Z_ISREF_P(old_data)) {
-							/* If the value is overwritten, remove old type source from ref. */
-							ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(old_data), info);
+						if (info) {
+							if (Z_ISREF_P(old_data)) {
+								/* If the value is overwritten, remove old type source from ref. */
+								ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(old_data), info);
+							}
+
+							if ((*var_hash)->ref_props) {
+								/* Remove old entry from ref_props table, if it exists. */
+								zend_hash_index_del(
+									(*var_hash)->ref_props, (zend_uintptr_t) old_data);
+							}
 						}
 						var_push_dtor(var_hash, old_data);
 						Z_TRY_DELREF_P(old_data);