php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81111 Serialization is unexpectedly allowed on anonymous classes with __serialize()
Submitted: 2021-06-05 18:20 UTC Modified: 2021-07-19 14:00 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: tandre@php.net Assigned:
Status: Open Package: *General Issues
PHP Version: 8.0.7 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please — but make sure to vote on the bug!
Your email address:
MUST BE VALID
Solve the problem:
29 - 15 = ?
Subscribe to this entry?

 
 [2021-06-05 18:20 UTC] 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


Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-06-20 00:53 UTC] 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-20 00:55 UTC] camporter1 at gmail dot com
I tried a fix for this that's a bit more basic (checking for zend_class_serialize_deny).
 [2021-07-17 15:21 UTC] 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-07-19 14:00 UTC] nikic@php.net
Fixed by https://github.com/php/php-src/commit/814a9327348b63ac15008c873f210e798d19fa26 (modulo conversion of remaining classes).
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Mon Sep 27 14:03:37 2021 UTC