Add ZEND_ACC_NOT_SERIALIZABLE flag

nikic · nikic · commit 814a9327348b · 2021-07-19T15:59:11.000+02:00
This prevents serialization and unserialization of a class and its children in a way that does not depend on the zend_class_serialize_deny and zend_class_unserialize_deny handlers that will be going away in PHP 9 together with the Serializable interface. In stubs, `@not-serializable` can be used to set this flag. This patch only uses the new flag for a handful of Zend classes, converting the remainder is left for later. Closes GH-7249. Fixes bug #81111.
diff --git a/NEWS b/NEWS
@@ -15,6 +15,10 @@ PHP                                                                        NEWS
 - Reflection:
  . Fixed bug #80097 (ReflectionAttribute is not a Reflector). (beberlei)
 
+- Standard:
+ . Fixed bug #81111 (Serialization is unexpectedly allowed on anonymous classes
+   with __serialize()). (Nikita)
+
 08 Jul 2021, PHP 8.1.0alpha3
 
 - Core:
diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS
@@ -47,6 +47,10 @@ PHP 8.1 INTERNALS UPGRADE NOTES
      implementations. Use ZEND_LONG_FMT and ZEND_ULONG_FMT instead.
   e. ZEND_ATOL() now returns the integer instead of assigning it as part of the
      macro. Replace ZEND_ATOL(i, s) with i = ZEND_ATOL(s).
+  f. Non-serializable classes should be indicated using the
+     ZEND_ACC_NOT_SERIALIZABLE (@not-serializable in stubs) rather than the
+     zend_class_(un)serialize_deny handlers. Support for the serialization
+     handlers will be dropped in the future.
 
 ========================
 2. Build system changes
diff --git a/Zend/tests/generators/errors/serialize_unserialize_error.phpt b/Zend/tests/generators/errors/serialize_unserialize_error.phpt
@@ -32,11 +32,11 @@ Stack trace:
 #0 %s(%d): serialize(Object(Generator))
 #1 {main}
 
+Exception: Unserialization of 'Generator' is not allowed in %s:%d
+Stack trace:
+#0 %s(%d): unserialize('O:9:"Generator"...')
+#1 {main}
 
-Warning: Erroneous data format for unserializing 'Generator' in %sserialize_unserialize_error.php on line %d
-
-Notice: unserialize(): Error at offset 19 of 20 bytes in %sserialize_unserialize_error.php on line %d
-bool(false)
 Exception: Unserialization of 'Generator' is not allowed in %s:%d
 Stack trace:
 #0 %s(%d): unserialize('C:9:"Generator"...')
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
@@ -640,8 +640,6 @@ void zend_register_closure_ce(void) /* {{{ */
 {
 	zend_ce_closure = register_class_Closure();
 	zend_ce_closure->create_object = zend_closure_new;
-	zend_ce_closure->serialize = zend_class_serialize_deny;
-	zend_ce_closure->unserialize = zend_class_unserialize_deny;
 
 	memcpy(&closure_handlers, &std_object_handlers, sizeof(zend_object_handlers));
 	closure_handlers.free_obj = zend_closure_free_storage;
diff --git a/Zend/zend_closures.stub.php b/Zend/zend_closures.stub.php
@@ -2,7 +2,10 @@
 
 /** @generate-class-entries */
 
-/** @strict-properties */
+/**
+ * @strict-properties
+ * @not-serializable
+ */
 final class Closure
 {
     private function __construct() {}
diff --git a/Zend/zend_closures_arginfo.h b/Zend/zend_closures_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 7c4df531cdb30ac4206f43f0d40098666466b9a6 */
+ * Stub hash: e3b480674671a698814db282c5ea34d438fe519d */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Closure___construct, 0, 0, 0)
 ZEND_END_ARG_INFO()
@@ -47,7 +47,7 @@ static zend_class_entry *register_class_Closure(void)
 
 	INIT_CLASS_ENTRY(ce, "Closure", class_Closure_methods);
 	class_entry = zend_register_internal_class_ex(&ce, NULL);
-	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES;
+	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES|ZEND_ACC_NOT_SERIALIZABLE;
 
 	return class_entry;
 }
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -7671,8 +7671,7 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel) /* {{{
 
 	if (UNEXPECTED((decl->flags & ZEND_ACC_ANON_CLASS))) {
 		/* Serialization is not supported for anonymous classes */
-		ce->serialize = zend_class_serialize_deny;
-		ce->unserialize = zend_class_unserialize_deny;
+		ce->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
 	}
 
 	if (extends_ast) {
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -238,7 +238,7 @@ typedef struct _zend_oparray_context {
 /* or IS_CONSTANT_VISITED_MARK                            |     |     |     */
 #define ZEND_CLASS_CONST_IS_CASE         (1 << 6)  /*     |     |     |  X  */
 /*                                                        |     |     |     */
-/* Class Flags (unused: 29...)                            |     |     |     */
+/* Class Flags (unused: 30...)                            |     |     |     */
 /* ===========                                            |     |     |     */
 /*                                                        |     |     |     */
 /* Special class types                                    |     |     |     */
@@ -301,6 +301,9 @@ typedef struct _zend_oparray_context {
 /* loaded from file cache to process memory               |     |     |     */
 #define ZEND_ACC_FILE_CACHED             (1 << 27) /*  X  |     |     |     */
 /*                                                        |     |     |     */
+/* Class cannot be serialized or unserialized             |     |     |     */
+#define ZEND_ACC_NOT_SERIALIZABLE        (1 << 29) /*  X  |     |     |     */
+/*                                                        |     |     |     */
 /* Function Flags (unused: 27-30)                         |     |     |     */
 /* ==============                                         |     |     |     */
 /*                                                        |     |     |     */
diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c
@@ -868,8 +868,6 @@ void zend_register_fiber_ce(void)
 {
 	zend_ce_fiber = register_class_Fiber();
 	zend_ce_fiber->create_object = zend_fiber_object_create;
-	zend_ce_fiber->serialize = zend_class_serialize_deny;
-	zend_ce_fiber->unserialize = zend_class_unserialize_deny;
 
 	zend_fiber_handlers = std_object_handlers;
 	zend_fiber_handlers.dtor_obj = zend_fiber_object_destroy;
diff --git a/Zend/zend_fibers.stub.php b/Zend/zend_fibers.stub.php
@@ -2,7 +2,10 @@
 
 /** @generate-class-entries */
 
-/** @strict-properties */
+/**
+ * @strict-properties
+ * @not-serializable
+ */
 final class Fiber
 {
     public function __construct(callable $callback) {}
diff --git a/Zend/zend_fibers_arginfo.h b/Zend/zend_fibers_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 7a3a7030f97d2c1e787499ef25341607841a607c */
+ * Stub hash: e82bbc8e81fe98873a9a5697a4b38e63a24379da */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Fiber___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 0)
@@ -79,7 +79,7 @@ static zend_class_entry *register_class_Fiber(void)
 
 	INIT_CLASS_ENTRY(ce, "Fiber", class_Fiber_methods);
 	class_entry = zend_register_internal_class_ex(&ce, NULL);
-	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES;
+	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES|ZEND_ACC_NOT_SERIALIZABLE;
 
 	return class_entry;
 }
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
@@ -1117,8 +1117,6 @@ void zend_register_generator_ce(void) /* {{{ */
 {
 	zend_ce_generator = register_class_Generator(zend_ce_iterator);
 	zend_ce_generator->create_object = zend_generator_create;
-	zend_ce_generator->serialize = zend_class_serialize_deny;
-	zend_ce_generator->unserialize = zend_class_unserialize_deny;
 	/* get_iterator has to be assigned *after* implementing the interface */
 	zend_ce_generator->get_iterator = zend_generator_get_iterator;
 
diff --git a/Zend/zend_generators.stub.php b/Zend/zend_generators.stub.php
@@ -2,7 +2,10 @@
 
 /** @generate-class-entries */
 
-/** @strict-properties */
+/**
+ * @strict-properties
+ * @not-serializable
+ */
 final class Generator implements Iterator
 {
     public function rewind(): void {}
diff --git a/Zend/zend_generators_arginfo.h b/Zend/zend_generators_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 06d4e8126db48fe8633ecd40b93904a0f9c59263 */
+ * Stub hash: 0af5e8985dd4645bf23490b8cec312f8fd1fee2e */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Generator_rewind, 0, 0, IS_VOID, 0)
 ZEND_END_ARG_INFO()
@@ -58,7 +58,7 @@ static zend_class_entry *register_class_Generator(zend_class_entry *class_entry_
 
 	INIT_CLASS_ENTRY(ce, "Generator", class_Generator_methods);
 	class_entry = zend_register_internal_class_ex(&ce, NULL);
-	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES;
+	class_entry->ce_flags |= ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES|ZEND_ACC_NOT_SERIALIZABLE;
 	zend_class_implements(class_entry, 1, class_entry_Iterator);
 
 	return class_entry;
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -1629,7 +1629,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			ce->ce_flags |= ZEND_ACC_EXPLICIT_ABSTRACT_CLASS;
 		}
 	}
-	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_USE_GUARDS);
+	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE);
 }
 /* }}} */
 
diff --git a/build/gen_stub.php b/build/gen_stub.php
@@ -1195,6 +1195,8 @@ class ClassInfo {
     public $isDeprecated;
     /** @var bool */
     public $isStrictProperties;
+    /** @var bool */
+    public $isNotSerializable;
     /** @var Name[] */
     public $extends;
     /** @var Name[] */
@@ -1217,6 +1219,7 @@ public function __construct(
         ?string $alias,
         bool $isDeprecated,
         bool $isStrictProperties,
+        bool $isNotSerializable,
         array $extends,
         array $implements,
         array $propertyInfos,
@@ -1228,6 +1231,7 @@ public function __construct(
         $this->alias = $alias;
         $this->isDeprecated = $isDeprecated;
         $this->isStrictProperties = $isStrictProperties;
+        $this->isNotSerializable = $isNotSerializable;
         $this->extends = $extends;
         $this->implements = $implements;
         $this->propertyInfos = $propertyInfos;
@@ -1318,6 +1322,10 @@ private function getFlagsAsString(): string
             $flags[] = "ZEND_ACC_NO_DYNAMIC_PROPERTIES";
         }
 
+        if ($this->isNotSerializable) {
+            $flags[] = "ZEND_ACC_NOT_SERIALIZABLE";
+        }
+
         return implode("|", $flags);
     }
 }
@@ -1625,6 +1633,7 @@ function parseClass(Name $name, Stmt\ClassLike $class, array $properties, array
     $alias = null;
     $isDeprecated = false;
     $isStrictProperties = false;
+    $isNotSerializable = false;
 
     if ($comment) {
         $tags = parseDocComment($comment);
@@ -1635,6 +1644,8 @@ function parseClass(Name $name, Stmt\ClassLike $class, array $properties, array
                 $isDeprecated = true;
             } else if ($tag->name === 'strict-properties') {
                 $isStrictProperties = true;
+            } else if ($tag->name === 'not-serializable') {
+                $isNotSerializable = true;
             }
         }
     }
@@ -1658,6 +1669,7 @@ function parseClass(Name $name, Stmt\ClassLike $class, array $properties, array
         $alias,
         $isDeprecated,
         $isStrictProperties,
+        $isNotSerializable,
         $extends,
         $implements,
         $properties,
diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c
@@ -2744,8 +2744,6 @@ PHP_MINIT_FUNCTION(spl_directory)
 	spl_filesystem_object_handlers.cast_object = spl_filesystem_object_cast;
 	spl_filesystem_object_handlers.dtor_obj = spl_filesystem_object_destroy_object;
 	spl_filesystem_object_handlers.free_obj = spl_filesystem_object_free_storage;
-	spl_ce_SplFileInfo->serialize = zend_class_serialize_deny;
-	spl_ce_SplFileInfo->unserialize = zend_class_unserialize_deny;
 
 	spl_ce_DirectoryIterator = register_class_DirectoryIterator(spl_ce_SplFileInfo, spl_ce_SeekableIterator);
 	spl_ce_DirectoryIterator->create_object = spl_filesystem_object_new;
diff --git a/ext/spl/spl_directory.stub.php b/ext/spl/spl_directory.stub.php
@@ -2,6 +2,7 @@
 
 /** @generate-class-entries */
 
+/** @not-serializable */
 class SplFileInfo implements Stringable
 {
     public function __construct(string $filename) {}
diff --git a/ext/spl/spl_directory_arginfo.h b/ext/spl/spl_directory_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 7c64c21963df5a11e902298eb5957b868c4b48cf */
+ * Stub hash: eab71d8a7172dba2dac3c6fa97b2064c7a99191f */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplFileInfo___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
@@ -496,6 +496,7 @@ static zend_class_entry *register_class_SplFileInfo(zend_class_entry *class_entr
 
 	INIT_CLASS_ENTRY(ce, "SplFileInfo", class_SplFileInfo_methods);
 	class_entry = zend_register_internal_class_ex(&ce, NULL);
+	class_entry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
 	zend_class_implements(class_entry, 1, class_entry_Stringable);
 
 	return class_entry;
diff --git a/ext/standard/tests/serialize/bug67072.phpt b/ext/standard/tests/serialize/bug67072.phpt
@@ -2,11 +2,12 @@
 Bug #67072 Echoing unserialized "SplFileObject" crash
 --FILE--
 <?php
-    echo unserialize('O:13:"SplFileObject":1:{s:9:"*filename";s:15:"/home/flag/flag";}');
+echo unserialize('O:13:"SplFileObject":1:{s:9:"*filename";s:15:"/home/flag/flag";}');
 ?>
 ===DONE==
 --EXPECTF--
-Warning: Erroneous data format for unserializing 'SplFileObject' in %sbug67072.php on line %d
-
-Notice: unserialize(): Error at offset 24 of 64 bytes in %sbug67072.php on line %d
-===DONE==
+Fatal error: Uncaught Exception: Unserialization of 'SplFileObject' is not allowed in %s:%d
+Stack trace:
+#0 %s(%d): unserialize('O:13:"SplFileOb...')
+#1 {main}
+  thrown in %s on line %d
diff --git a/ext/standard/tests/serialize/bug81111.phpt b/ext/standard/tests/serialize/bug81111.phpt
@@ -0,0 +1,53 @@
+--TEST--
+Bug #81111: Serialization is unexpectedly allowed on anonymous classes with __serialize()
+--FILE--
+<?php
+
+class MySplFileInfo extends SplFileInfo {
+    public function __serialize() { return []; }
+    public function __unserialize($value) { return new self('file'); }
+}
+
+try {
+    serialize(new MySplFileInfo(__FILE__));
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+$anon = new class () {
+    public function __serialize() { return []; }
+    public function __unserialize($value) { }
+};
+
+try {
+    serialize($anon);
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    unserialize("O:13:\"MySplFileInfo\":0:{}");
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    unserialize("C:13:\"MySplFileInfo\":0:{}");
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+$name = $anon::class;
+try {
+    unserialize("O:" . strlen($name) . ":\"" . $name . "\":0:{}");
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECTF--
+Serialization of 'MySplFileInfo' is not allowed
+Serialization of 'class@anonymous' is not allowed
+Unserialization of 'MySplFileInfo' is not allowed
+Unserialization of 'MySplFileInfo' is not allowed
+
+Notice: unserialize(): Error at offset 0 of %d bytes in %s on line %d
diff --git a/ext/standard/var.c b/ext/standard/var.c
@@ -27,6 +27,7 @@
 #include "basic_functions.h"
 #include "php_incomplete_class.h"
 #include "zend_enum.h"
+#include "zend_exceptions.h"
 /* }}} */
 
 struct php_serialize_data {
@@ -1058,6 +1059,12 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
 				bool incomplete_class;
 				uint32_t count;
 
+				if (ce->ce_flags & ZEND_ACC_NOT_SERIALIZABLE) {
+					zend_throw_exception_ex(NULL, 0, "Serialization of '%s' is not allowed",
+						ZSTR_VAL(ce->name));
+					return;
+				}
+
 				if (ce->ce_flags & ZEND_ACC_ENUM) {
 					PHP_CLASS_ATTRIBUTES;
 
diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re
@@ -18,6 +18,7 @@
 #include "ext/standard/php_var.h"
 #include "php_incomplete_class.h"
 #include "zend_portability.h"
+#include "zend_exceptions.h"
 
 /* {{{ reference-handling for unserializer: var_* */
 #define VAR_ENTRIES_MAX 1018     /* 1024 - offsetof(php_unserialize_data, entries) / sizeof(void*) */
@@ -1267,6 +1268,13 @@ object ":" uiv ":" ["]	{
 
 	*p = YYCURSOR;
 
+	if (ce->ce_flags & ZEND_ACC_NOT_SERIALIZABLE) {
+		zend_throw_exception_ex(NULL, 0, "Unserialization of '%s' is not allowed",
+			ZSTR_VAL(ce->name));
+		zend_string_release_ex(class_name, 0);
+		return 0;
+	}
+
 	if (custom_object) {
 		int ret;
 

Original file line number	Diff line number	Diff line change
`@@ -640,8 +640,6 @@ void zend_register_closure_ce(void) /* {{{ */`
`640`	`640`	`{`
`641`	`641`	`zend_ce_closure = register_class_Closure();`
`642`	`642`	`zend_ce_closure->create_object = zend_closure_new;`
`643`		`- zend_ce_closure->serialize = zend_class_serialize_deny;`
`644`		`- zend_ce_closure->unserialize = zend_class_unserialize_deny;`
`645`	`643`
`646`	`644`	`memcpy(&closure_handlers, &std_object_handlers, sizeof(zend_object_handlers));`
`647`	`645`	`closure_handlers.free_obj = zend_closure_free_storage;`
Original file line number	Diff line number	Diff line change
`@@ -7671,8 +7671,7 @@ void zend_compile_class_decl(znode result, zend_ast ast, bool toplevel) /* {{{`
`7671`	`7671`
`7672`	`7672`	`if (UNEXPECTED((decl->flags & ZEND_ACC_ANON_CLASS))) {`
`7673`	`7673`	`/* Serialization is not supported for anonymous classes */`
`7674`		`- ce->serialize = zend_class_serialize_deny;`
`7675`		`- ce->unserialize = zend_class_unserialize_deny;`
	`7674`	`+ ce->ce_flags \|= ZEND_ACC_NOT_SERIALIZABLE;`
`7676`	`7675`	`}`
`7677`	`7676`
`7678`	`7677`	`if (extends_ast) {`
Original file line number	Diff line number	Diff line change
`@@ -868,8 +868,6 @@ void zend_register_fiber_ce(void)`
`868`	`868`	`{`
`869`	`869`	`zend_ce_fiber = register_class_Fiber();`
`870`	`870`	`zend_ce_fiber->create_object = zend_fiber_object_create;`
`871`		`- zend_ce_fiber->serialize = zend_class_serialize_deny;`
`872`		`- zend_ce_fiber->unserialize = zend_class_unserialize_deny;`
`873`	`871`
`874`	`872`	`zend_fiber_handlers = std_object_handlers;`
`875`	`873`	`zend_fiber_handlers.dtor_obj = zend_fiber_object_destroy;`