Fix GH-9186 @strict-properties can be bypassed using unserialization

php · Aug 17, 2022 · a5d030e · a5d030e
1 parent 5f8993b
commit a5d030e
Show file tree

Hide file tree

Showing 20 changed files with 140 additions and 35 deletions.
diff --git a/Zend/tests/gc_043.phpt b/Zend/tests/gc_043.phpt
@@ -8,7 +8,8 @@ STR;
 var_dump(unserialize($s));
 gc_collect_cycles();
 ?>
---EXPECT--
+--EXPECTF--
+Deprecated: Creation of dynamic property RegexIterator::$5 is deprecated in %s on line %d
 object(stdClass)#1 (2) {
   ["5"]=>
   object(SplStack)#2 (2) {

diff --git a/Zend/tests/readonly_classes/readonly_class_unserialize_error.phpt b/Zend/tests/readonly_classes/readonly_class_unserialize_error.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Fix GH-9186 Readonly classes can have dynamic properties created by unserialize()
+--FILE--
+<?php
+
+readonly class C {}
+
+try {
+    $readonly = unserialize('O:1:"C":1:{s:1:"x";b:1;}');
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+Cannot create dynamic property C::$x
diff --git a/Zend/zend_API.c b/Zend/zend_API.c
@@ -1586,6 +1586,28 @@ ZEND_API void object_properties_init_ex(zend_object *object, HashTable *properti
 }
 /* }}} */
 
+ZEND_API zend_never_inline void zend_forbidden_dynamic_property(zend_class_entry *ce, zend_string *member) {
+	zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
+		ZSTR_VAL(ce->name), ZSTR_VAL(member));
+}
+
+ZEND_API zend_never_inline bool zend_deprecated_dynamic_property(zend_object *obj, zend_string *member) {
+	GC_ADDREF(obj);
+	zend_error(E_DEPRECATED, "Creation of dynamic property %s::$%s is deprecated",
+		ZSTR_VAL(obj->ce->name), ZSTR_VAL(member));
+	if (UNEXPECTED(GC_DELREF(obj) == 0)) {
+		zend_class_entry *ce = obj->ce;
+		zend_objects_store_del(obj);
+		if (!EG(exception)) {
+			/* We cannot continue execution and have to throw an exception */
+			zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
+				ZSTR_VAL(ce->name), ZSTR_VAL(member));
+		}
+		return 0;
+	}
+	return 1;
+}
+
 ZEND_API void object_properties_load(zend_object *object, HashTable *properties) /* {{{ */
 {
 	zval *prop, tmp;
@@ -1627,13 +1649,31 @@ ZEND_API void object_properties_load(zend_object *object, HashTable *properties)
 					zend_hash_update(object->properties, key, &tmp);
 				}
 			} else {
+				if (UNEXPECTED(object->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
+					zend_forbidden_dynamic_property(object->ce, key);
+					return;
+				} else if (!(object->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES)) {
+					if (!zend_deprecated_dynamic_property(object, key)) {
+						return;
+					}
+				}
+
 				if (!object->properties) {
 					rebuild_object_properties(object);
 				}
 				prop = zend_hash_update(object->properties, key, prop);
 				zval_add_ref(prop);
 			}
 		} else {
+			if (UNEXPECTED(object->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
+				zend_forbidden_dynamic_property(object->ce, key);
+				return;
+			} else if (!(object->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES)) {
+				if (!zend_deprecated_dynamic_property(object, key)) {
+					return;
+				}
+			}
+
 			if (!object->properties) {
 				rebuild_object_properties(object);
 			}

diff --git a/Zend/zend_API.h b/Zend/zend_API.h
@@ -522,6 +522,8 @@ ZEND_API zend_result object_init_ex(zval *arg, zend_class_entry *ce);
 ZEND_API zend_result object_and_properties_init(zval *arg, zend_class_entry *ce, HashTable *properties);
 ZEND_API void object_properties_init(zend_object *object, zend_class_entry *class_type);
 ZEND_API void object_properties_init_ex(zend_object *object, HashTable *properties);
+ZEND_API zend_never_inline void zend_forbidden_dynamic_property(zend_class_entry *ce, zend_string *member);
+ZEND_API zend_never_inline bool zend_deprecated_dynamic_property(zend_object *obj, zend_string *member);
 ZEND_API void object_properties_load(zend_object *object, HashTable *properties);
 
 ZEND_API void zend_merge_properties(zval *obj, HashTable *properties);

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
@@ -275,30 +275,6 @@ static ZEND_COLD zend_never_inline void zend_bad_property_name(void) /* {{{ */
 }
 /* }}} */
 
-static ZEND_COLD zend_never_inline void zend_forbidden_dynamic_property(
-		zend_class_entry *ce, zend_string *member) {
-	zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
-		ZSTR_VAL(ce->name), ZSTR_VAL(member));
-}
-
-static ZEND_COLD zend_never_inline bool zend_deprecated_dynamic_property(
-		zend_object *obj, zend_string *member) {
-	GC_ADDREF(obj);
-	zend_error(E_DEPRECATED, "Creation of dynamic property %s::$%s is deprecated",
-		ZSTR_VAL(obj->ce->name), ZSTR_VAL(member));
-	if (UNEXPECTED(GC_DELREF(obj) == 0)) {
-		zend_class_entry *ce = obj->ce;
-		zend_objects_store_del(obj);
-		if (!EG(exception)) {
-			/* We cannot continue execution and have to throw an exception */
-			zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
-				ZSTR_VAL(ce->name), ZSTR_VAL(member));
-		}
-		return 0;
-	}
-	return 1;
-}
-
 static ZEND_COLD zend_never_inline void zend_readonly_property_modification_scope_error(
 		zend_class_entry *ce, zend_string *member, zend_class_entry *scope, const char *operation) {
 	zend_throw_error(NULL, "Cannot %s readonly property %s::$%s from %s%s",

diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c
@@ -365,6 +365,9 @@ PHP_METHOD(Random_Engine_Mt19937, __unserialize)
 		RETURN_THROWS();
 	}
 	object_properties_load(&engine->std, Z_ARRVAL_P(t));
+	if (EG(exception)) {
+		RETURN_THROWS();
+	}
 
 	/* state */
 	t = zend_hash_index_find(d, 1);

diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c
@@ -90,7 +90,7 @@ PHP_METHOD(Random_Randomizer, __construct)
 
 /* {{{ Generate positive random number */
 PHP_METHOD(Random_Randomizer, nextInt)
-{	
+{
 	php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS);
 	uint64_t result;
 
@@ -104,7 +104,7 @@ PHP_METHOD(Random_Randomizer, nextInt)
 		zend_throw_exception(random_ce_Random_RandomException, "Generated value exceeds size of int", 0);
 		RETURN_THROWS();
 	}
-	
+
 	RETURN_LONG((zend_long) (result >> 1));
 }
 /* }}} */
@@ -278,6 +278,9 @@ PHP_METHOD(Random_Randomizer, __unserialize)
 		RETURN_THROWS();
 	}
 	object_properties_load(&randomizer->std, Z_ARRVAL_P(members_zv));
+	if (EG(exception)) {
+		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)) {

diff --git a/ext/random/tests/03_randomizer/gh_9186_unserialize.phpt b/ext/random/tests/03_randomizer/gh_9186_unserialize.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Fix GH-9186 @strict-properties can be bypassed using unserialization
+--FILE--
+<?php
+
+try {
+    unserialize('O:17:"Random\Randomizer":1:{i:0;a:2:{s:3:"foo";N;s:6:"engine";O:32:"Random\Engine\Xoshiro256StarStar":2:{i:0;a:0:{}i:1;a:4:{i:0;s:16:"7520fbc2d6f8de46";i:1;s:16:"84d2d2b9d7ba0a34";i:2;s:16:"d975f36db6490b32";i:3;s:16:"c19991ee16785b94";}}}}');
+} catch (Error $error) {
+    echo $error->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+Cannot create dynamic property Random\Randomizer::$foo
diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
@@ -1740,6 +1740,9 @@ PHP_METHOD(ArrayObject, __unserialize)
 	}
 
 	object_properties_load(&intern->std, Z_ARRVAL_P(members_zv));
+	if (EG(exception)) {
+		RETURN_THROWS();
+	}
 
 	if (iterator_class_zv && Z_TYPE_P(iterator_class_zv) == IS_STRING) {
 		zend_class_entry *ce = zend_lookup_class(Z_STR_P(iterator_class_zv));

diff --git a/ext/standard/basic_functions.stub.php b/ext/standard/basic_functions.stub.php
@@ -8,6 +8,7 @@
  */
 const M_E = 2.718281828459045;
 
+#[AllowDynamicProperties]
 final class __PHP_Incomplete_Class
 {
 }

diff --git a/ext/standard/basic_functions_arginfo.h b/ext/standard/basic_functions_arginfo.h
diff --git a/ext/standard/tests/serialize/bug49649.phpt b/ext/standard/tests/serialize/bug49649.phpt
@@ -33,7 +33,12 @@ class Foo
 $class = unserialize(base64_decode($serialized));
 var_dump($class);
 ?>
---EXPECT--
+--EXPECTF--
+Deprecated: Creation of dynamic property Foo::$ is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$ is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$notThere is deprecated in %s on line %d
 object(Foo)#1 (4) {
   ["public"]=>
   int(3)

diff --git a/ext/standard/tests/serialize/bug49649_1.phpt b/ext/standard/tests/serialize/bug49649_1.phpt
@@ -33,7 +33,12 @@ class Foo
 $class = unserialize(base64_decode($serialized));
 var_dump($class);
 ?>
---EXPECT--
+--EXPECTF--
+Deprecated: Creation of dynamic property Foo::$ is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$public is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$notThere is deprecated in %s on line %d
 object(Foo)#1 (4) {
   ["public":protected]=>
   int(3)

diff --git a/ext/standard/tests/serialize/bug49649_2.phpt b/ext/standard/tests/serialize/bug49649_2.phpt
@@ -33,7 +33,12 @@ class Foo
 $class = unserialize(base64_decode($serialized));
 var_dump($class);
 ?>
---EXPECT--
+--EXPECTF--
+Deprecated: Creation of dynamic property Foo::$ is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$public is deprecated in %s on line %d
+
+Deprecated: Creation of dynamic property Foo::$notThere is deprecated in %s on line %d
 object(Foo)#1 (4) {
   ["public":"Foo":private]=>
   int(3)

diff --git a/ext/standard/tests/serialize/bug62836_1.phpt b/ext/standard/tests/serialize/bug62836_1.phpt
@@ -5,7 +5,7 @@ Bug #62836 (Seg fault or broken object references on unserialize())
 $serialized_object='O:1:"A":4:{s:1:"b";O:1:"B":0:{}s:2:"b1";r:2;s:1:"c";O:1:"B":0:{}s:2:"c1";r:4;}';
 spl_autoload_register(function ($name) {
     unserialize("i:4;");
-    eval("class $name {} ");
+    eval("#[AllowDynamicProperties] class $name {} ");
 });
 
 print_r(unserialize($serialized_object));

diff --git a/ext/standard/tests/serialize/bug62836_2.phpt b/ext/standard/tests/serialize/bug62836_2.phpt
@@ -8,7 +8,7 @@ ini_set('unserialize_callback_func','mycallback');
 
 function mycallback($classname) {
     unserialize("i:4;");
-    eval ("class $classname {} ");
+    eval ("#[AllowDynamicProperties] class $classname {} ");
 }
 
 print_r(unserialize($serialized_object));

diff --git a/ext/standard/tests/serialize/bug72663.phpt b/ext/standard/tests/serialize/bug72663.phpt
@@ -3,6 +3,7 @@ Bug #72663 (1): Don't call __destruct if __wakeup not called or fails
 --FILE--
 <?php
 
+#[AllowDynamicProperties]
 class Test1 {
     public function __wakeup() {
         echo "Wakeup\n";
@@ -12,6 +13,7 @@ class Test1 {
     }
 }
 
+#[AllowDynamicProperties]
 class Test2 {
     public function __wakeup() {
         throw new Exception('Unserialization forbidden');

diff --git a/ext/standard/tests/serialize/serialization_objects_incomplete.phpt b/ext/standard/tests/serialize/serialization_objects_incomplete.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Check behaviour of incomplete class
+--FILE--
+<?php
+$incomplete = unserialize('O:1:"C":1:{s:1:"p";i:1;}');
+var_dump($incomplete);
+
+?>
+--EXPECT--
+object(__PHP_Incomplete_Class)#1 (2) {
+  ["__PHP_Incomplete_Class_Name"]=>
+  string(1) "C"
+  ["p"]=>
+  int(1)
+}
diff --git a/ext/standard/tests/serialize/unserialize_overwrite_undeclared_protected.phpt b/ext/standard/tests/serialize/unserialize_overwrite_undeclared_protected.phpt
@@ -12,7 +12,8 @@ O:4:"Test":2:{s:4:"\0*\0x";N;s:4:"\0*\0x";N;}
 STR;
 var_dump(unserialize($str));
 ?>
---EXPECT--
+--EXPECTF--
+Deprecated: Creation of dynamic property Test::$ is deprecated in %s on line %d
 object(Test)#1 (2) {
   ["foo"]=>
   NULL

diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re
@@ -638,6 +638,15 @@ declared_property:
 					}
 				}
 			} else {
+				if (UNEXPECTED(obj->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
+					zend_forbidden_dynamic_property(obj->ce, Z_STR_P(&key));
+					goto failure;
+				} else if (!(obj->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES)) {
+					if (!zend_deprecated_dynamic_property(obj, Z_STR_P(&key))) {
+						goto failure;
+					}
+				}
+
 				int ret = is_property_visibility_changed(obj->ce, &key);
 
 				if (EXPECTED(!ret)) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -8,6 +8,7 @@ @@
      */
     const M_E = 2.718281828459045;
+    #[AllowDynamicProperties]
     final class __PHP_Incomplete_Class
     {
     }
@@ Expand Down @@