Doc #68099 [Asn->Wfx]: Unable to unserialize data after implementing Serializable
| From: | tyrael@php.net | Date: | Mon, 06 Apr 2015 09:24:20 +0000 |
| Subject: | Doc #68099 [Asn->Wfx]: Unable to unserialize data after implementing Serializable | ||
| References: | 1 | Groups: | php.doc.bugs |
| Request: | Send a blank email to doc-bugs+get-12107@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: Assigned
+Status: Wont fix
Type: Documentation Problem
Package: *General Issues
PHP Version: 5.6.1RC1
Assigned To: tyrael
Block user comment: N
Private report: N
Previous Comments:
------------------------------------------------------------------------
[2014-10-15 18:32:52] tyrael@php.net
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.
------------------------------------------------------------------------
[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