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

Edit report at https://bugs.php.net/bug.php?id=74436&edit=1 ID: 74436 User updated by: tom at altrooz dot com Reported by: tom at altrooz dot com Summary: Mixing Serializable and __wakeup() causes conflict Status: Open Type: Bug Package: Class/Object related Operating System: Centos 6 PHP Version: 5.6.30 Block user comment: N Private report: N New Comment: 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. Previous Comments: ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ [2017-04-14 17:49:35] danack@php.net The version of PHP you've reported this bug for is only receiving security updates. If you can't provide a reproduce case that shows the problem in PHP 7, it is unlikely that anyone would look at this. ------------------------------------------------------------------------ [2017-04-13 20:41:57] tom at altrooz dot com Description: ------------ This issue was introduced in 5.6.30, and does not exist in 5.6.29. Unfortunately the data structures that showed this effect are too complicated to include, and my effort to create a smaller reproducable case was not successful. But here's the story: *) A class implemented Serializable *) It has a member of type SplFixedArray, which has a __wakeup() function *) The container held other class instances that did not override the default serialization behavior *) Some of those classes included instances of ImmutableDateTime, which also has a __wakeup() function When unserializing the top-level class, the __wakeup() function on the SplFixedArray and ImmutableDateTime instances did not get called. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=74436&edit=1