Doc #68099 [Opn]: Unable to unserialize data after implementing Serializable

From: Date: Wed, 15 Oct 2014 18:32:52 +0000
Subject: Doc #68099 [Opn]: Unable to unserialize data after implementing Serializable
References: 1  Groups: php.doc.bugs 
Request: Send a blank email to doc-bugs+get-11553@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=68099&edit=1 ID: 68099 Updated by: tyrael@php.net Reported by: manuel-php at mausz dot at Summary: Unable to unserialize data after implementing Serializable Status: Open Type: Documentation Problem Package: *General Issues PHP Version: 5.6.1RC1 Block user comment: N Private report: N New Comment: I agree that this is a BC break, but the original behavior (that we allow the old serialize format to be used for classes using the new format) was never documented nor officially supported, but I would still vote to keep it for now if this behavior couldn't cause potential security problems, https://bugs.php.net/bug.php?id=67072 was the initial bugreport which made us realize that this trick which allows the unserialize call to be bypassed for classes using the new serialize format(classes which are implementing Serialize) can be potentially used to cause segfaults or even more dangerous problems, and there are lots of php apps out there which allows unserialization of arbitrary so that plugging this hole was warranted. if you have an idea how to fix this "properly", I'm all ears, we had some really lengthy discussion on the mailing list and couldn't come up with a better approach. (for the record we introduced a less strict check for 5.4 and 5.5 where we only reject the unserialization of the Serializable classes with the old format where the class is or extending an internal class which can cause memory corruptions like the one described in the original bugreport). if you still feel that this is too much of a BC impact or if you have a better suggestion, please bring this on the internals@ mailing list, where more people can join the discussion. Previous Comments: ------------------------------------------------------------------------ [2014-10-15 16:50:04] manuel-php at mausz dot at Hi Ferenc, please don't consider the test script to be a something that's in production. In fact we stumbled across this while upgrading a webserver running horde webmail. Horde upgraded a bunch of classes to implement Serializable (https://github.com/horde/horde/commit/bc36d596a868602f525d2cc57e9de1155a23456e) about a year back. So any data that was serialized before this change is broken with PHP 5.6. I consider this a major BC break for a minor PHP version, something that is hard to find and almost impossible to fix at the appplication level. You can catch the failed unserialize()-call but you can't unserialize/convert the data unless you implement the serialize format in PHP. Also c2acdbdd3deb6787329bf0aca8ab0c04ace2a50c doesn't reintroduce #67072. I fail to see why this can't be reverted (or fixed correctly). ------------------------------------------------------------------------ [2014-10-15 16:25:16] tyrael@php.net hi, classes implementing the Serializable interface are using a different serialize format than used for other classes. unfortunately when this new format was introduced, there were no checks implemented to enforce this behavior, so it was possible to unserialize an instance implementing the Serialize interface using the old format, which bypassed the call for the unserialize method declared for that class. with 5.6 we implemented these checks to enforce that the unserialize calls can't be bypassed using the old serialize format. while what you are doing in your example is pretty risky, as it is prone to result in an bogus instance(the only case where it would produce the correct result is that if your class' unserialize handler writes the same properties and does no other transformation on the data), I think you are still right that we should explicitly state this behavior change in the migration guide(currently we only mention the change in the unserialize manual: http://php.net/manual/en/function.unserialize.php ). ------------------------------------------------------------------------ [2014-09-25 20:31:55] manuel-php at mausz dot at Reverting c2acdbdd3deb6787329bf0aca8ab0c04ace2a50c fixes this issue ------------------------------------------------------------------------ [2014-09-25 16:23:51] manuel-php at mausz dot at Description: ------------ Consider the following case: * User implements class Foo which gets serialized and unserialized during daily use. Serialized data is stored somewhere. * User changes class Foo to implement Serializable * User still wants to transparently unserialize his old data This worked in PHP5.5 and below. It's broken in PHP 5.6.0 and above. If this is intended than this is a major BC break not mentioned in the upgrade guidelines. Test script: --------------- <?php error_reporting(E_ALL); # export ... simulates exporting old data where class Foo didn't implement Serializable $mode = (isset($argv[1]) && $argv[1] === 'import') ? 'import' : 'export'; if ($mode === 'export') { class Foo { protected $foo = null; public function __construct() { $this->foo = "something"; } } $foo = new Foo(); $data = serialize($foo); file_put_contents('/tmp/phpbug', $data); } else { class Foo implements Serializable { protected $foo = null; public function __construct() { $this->foo = "something"; } public function serialize() { echo __CLASS__ . "::serialize called\n"; return $this->foo; } public function unserialize($data) { echo __CLASS__ . "::unserialize called\n"; $this->foo = $data; } } $data = file_get_contents('/tmp/phpbug'); $foo = unserialize($data); if ($foo instanceof Foo) echo "Thumbs up!\n"; else echo "Unserialize FAILED\n"; } Expected result: ---------------- # php5.5 serialize.php export; php5.5 serialize.php import Thumbs up! Actual result: -------------- ./php-src-PHP-5.6.1/sapi/cli/php serialize.php export ; ./php-src-PHP-5.6.1/sapi/cli/php serialize.php import Warning: Erroneous data format for unserializing 'Foo' in /home/manuel/serialize.php on line 49 Notice: unserialize(): Error at offset 13 of 43 bytes in /home/manuel/serialize.php on line 49 Unserialize FAILED ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=68099&edit=1

« previous php.doc.bugs (#11553) next »