Bug #81111 [Opn]: Serialization is unexpectedly allowed on anonymous classes with __serialize()

Edit report at https://bugs.php.net/bug.php?id=81111&edit=1 ID: 81111 Updated by: nikic@php.net Reported by: tandre@php.net Summary: Serialization is unexpectedly allowed on anonymous classes with __serialize() Status: Open Type: Bug Package: *General Issues PHP Version: 8.0.7 Block user comment: N Private report: N New Comment: Fixed by https://github.com/php/php-src/commit/814a9327348b63ac15008c873f210e798d19fa26 (modulo conversion of remaining classes). Previous Comments: ------------------------------------------------------------------------ [2021-07-17 15:21:46] tandre@php.net The following pull request has been associated: Patch Name: Add ZEND_ACC_NOT_SERIALIZABLE flag On GitHub: https://github.com/php/php-src/pull/7249 Patch: https://github.com/php/php-src/pull/7249.patch ------------------------------------------------------------------------ [2021-06-20 00:55:21] camporter1 at gmail dot com I tried a fix for this that's a bit more basic (checking for zend_class_serialize_deny). ------------------------------------------------------------------------ [2021-06-20 00:53:47] camporter1 at gmail dot com The following pull request has been associated: Patch Name: Fix bug 81111: Prevent serialization of anonymous classes On GitHub: https://github.com/php/php-src/pull/7176 Patch: https://github.com/php/php-src/pull/7176.patch ------------------------------------------------------------------------ [2021-06-05 18:20:23] tandre@php.net Description: ------------ Related to https://bugs.php.net/bug.php?id=69761 This may also affect internal classes that use zend_class_serialize_deny. It may make sense to add some sort of bit flag instead to forbid serialization in a class and all of its subclasses. This affects php 7.4 and newer versions. Because some libraries may be unintentionally or intentionally relying on this (e.g. to serialize but not unserialize), my preference would be fixing it in 8.1. (I doubt this combination is commonly used but haven't looked into it) - If an anonymous class extends another class then it might end up declaring __serialize https://wiki.php.net/rfc/custom_object_serialization added support for __serialize in 7.4 but did not specify behavior for anonymous classes. The anonymous class name (with a null character followed by a unique identifier) is typically not deterministic, so I believe that it should be an error to serialize an anonymous class whether or not __serialize is defined. - Note that it is possible for a class to extend an anonymous class through the use of class_alias on get_class($obj). Forbidding serializing subclasses of anonymous classes is probably out of scope, since this is extremely uncommon and would be fine if the anonymous class had no private properties. ``` // in Zend/zend_compile.c, 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; } ``` It may be better to check if the bitflag is set on zend_class_entry in (ce->ce_flags & ZEND_ACC_ANON_CLASS) when serializing and unserializing (in ext/standard/var.c and ext/standard/var_unserializer.re) Noticed while working on https://github.com/igbinary/igbinary/issues/251 Test script: --------------- <?php function check_serialize_throws($obj) { try { var_dump(serialize($obj)); } catch (Throwable $e) { echo "Caught: " . $e->getMessage() . "\n"; } } check_serialize_throws(new class () {}); check_serialize_throws(new class () { public function __serialize() { return []; } public function __unserialize($value) { } }); check_serialize_throws(new class () implements Serializable { public function serialize() { return ''; } public function unserialize(string $ser) { return new self(); } }); Expected result: ---------------- All calls should result in the call to serialize throwing and "Caught: Serialization of 'Serializable@anonymous' is not allowed" being emitted, including on an anonymous class defining the __serialize() magic method. Actual result: -------------- When an anonymous class has the method __serialize, it can be serialized. Caught: Serialization of 'class@anonymous' is not allowed string(46) "O:34:"class@anonymousphp shell code:1$4":0:{}" Deprecated: The Serializable interface is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in php shell code on line 1 Caught: Serialization of 'Serializable@anonymous' is not allowed ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=81111&edit=1