Bug #74436 [Opn->Wfx]: Mixing Serializable and __wakeup() causes conflict

Edit report at https://bugs.php.net/bug.php?id=74436&edit=1 ID: 74436 Updated by: cmb@php.net Reported by: tom at altrooz dot com Summary: Mixing Serializable and __wakeup() causes conflict -Status: Open +Status: Wont fix Type: Bug Package: Class/Object related Operating System: Centos 6 PHP Version: 5.6.30 -Assigned To: +Assigned To: cmb Block user comment: N Private report: N New Comment: Sorry, if that change caused any harm, but a *vulnerability* had to be fixed, and due to fundamental issues of Serializable that was not possible without a BC break. The likely minor break has been chosen, and this will stay this way. The good news: as of PHP 7.4.0, there is a third serialization mechanism[1] which does not have these fundamental issues. I suggest to upgrade relevant code to use this new mechanism as soon as possible. Thanks. [1] <https://wiki.php.net/rfc/custom_object_serialization> Previous Comments: ------------------------------------------------------------------------ [2017-06-01 10:02:46] nikic@php.net Related To: Bug #74687 ------------------------------------------------------------------------ [2017-04-17 17:40:01] tom at altrooz dot com It does appear that this has significantly affected users of the CakePHP community. See https://github.com/cakephp/cakephp/issues/10111 for a sense of how much developer time is being wasted, and also with PHP 7.x. ------------------------------------------------------------------------ [2017-04-14 19:38:36] tom at altrooz dot com That is valuable information and one would hope that future security patches don't change application behavior. I'm sure you can now create a simple test case - The code that failed here is in the CakePHP framework, and has this innocuous construct (edited for brevity): class XXX implements Serializable { private $contents; // expected to be an instance of SplFixedArray private $contents_size; public function serialize() { return serialize($this->contents); } public function unserialize($data) { $this->contents = unserialize($data); $this->contents_size = count($this->contents); } } It is expected that when the SplFixedArray instance is rehydrated, it is now complete and can be referenced. In 5.6.29 (and of course every version before) this works correctly, but in 5.6.30 the count(SplFixedArray) returns 0 because __wakeup() has not been called on it. *All* object-oriented languages guarantee that class instances are completely instantiated before they are able to be accessed by the developer. ------------------------------------------------------------------------ [2017-04-14 19:18:52] nikic@php.net To clarify, at which point did you check whether the __wakeup() has been called? Did you check this after the unserialization finished completely, or did you check this in the Serializable::unserialize() method of your class? If the latter, then this is indeed how unserialization behaves now: __wakeup() will only be called at the very end of the unserialization, which also means that Serializable::unserialize() methods (unfortunately) will see the objects prior to __wakeup(). The objects will be woken up at a later point though. If this is the problem you're experiencing, I'm sorry to say that this is unlikely to change -- delaying __wakeup() calls until after ::unserialize() is an important part of the security issue being fixed here. The only alternative to this would be to remove the serialization context sharing for Serialization classes, which we believe to be a larger backwards compatibility break. ------------------------------------------------------------------------ [2017-04-14 17:58:18] tom at altrooz dot com And introducing a new significant flaw in security update 5.6.30 isn't a concern? As stated, I was not able to create a reproducable case but it seems a code review of http://bugs.php.net/70213 and http://bugs.php.net/73825 might be wise. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=74436 -- Edit this bug report at https://bugs.php.net/bug.php?id=74436&edit=1