Bug #62852 [Com]: Unserialize Invalid Date causes crash

From: Date: Tue, 05 Nov 2013 03:00:00 +0000
Subject: Bug #62852 [Com]: Unserialize Invalid Date causes crash
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-182598@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=62852&edit=1

 ID:                 62852
 Comment by:         mkwan at corp dot oodle dot com
 Reported by:        kasper at webmasteren dot eu
 Summary:            Unserialize Invalid Date causes crash
 Status:             Closed
 Type:               Bug
 Package:            Reproducible crash
 Operating System:   windows, linux
 PHP Version:        Irrelevant
 Assigned To:        laruence
 Block user comment: N
 Private report:     N

 New Comment:

According to the documentation, if "the passed string is not unserializeable, FALSE is returned
and E_NOTICE is issued."
http://php.net/manual/en/function.unserialize.php

Why is it that if the string happens to looks like a DateTime, instead an unrecoverable E_ERROR is
issued?


Previous Comments:
------------------------------------------------------------------------
[2013-04-19 20:42:52] webmaster at thedigitalorchard dot ca

My [ugly] workaround for this problem is to manually replace instances of 
serialized DateTime objects with a fake, non-existent class name, which avoids 
this crash.

$str = 'O:8:"DateTime":0:{}';
$str = str_replace('O:8:"DateTime"', 'O:12:"PHP_DateTime"',
$str);

Of course, if the serialized data needed to be recovered, an alternate approach 
would be needed. In my own case, I want to be discarding this object. I'm hoping 
this issue that ran into is an unforeseen issue with this latest bug fix, and a 
proper fix can be made in a future update. I don't like adding in workarounds. :-)

------------------------------------------------------------------------
[2013-04-19 20:28:26] webmaster at thedigitalorchard dot ca

I'm getting an error since this bug was "fixed". In one of my databases, a 
DateTime object was inadvertently serialized as a child object. Now, with this bug 
fix, I'm getting the following error.

The serialized object is represented by this short string:

O:8:"DateTime":0:{}

Running that through unserialize presents this error:
"Invalid serialization data for DateTime object"

I'm unable to catch this error and handle it gracefully (ie. ignoring this object 
unserialization entirely).

------------------------------------------------------------------------
[2013-03-15 20:31:10] ab@php.net

Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f8b91d9acff10ede7bd3f2bc631794a3abef8ff7
Log: Fixed bug #62852 Unserialize Invalid Date crash

------------------------------------------------------------------------
[2013-03-15 08:15:44] ab@php.net

This is also related to bug #53437, where the current implementation suggests 
E_ERROR. The fix for ticket should have exactly the same handling the other one 
has.

------------------------------------------------------------------------
[2013-03-14 07:50:26] ab@php.net

Looks like there is no other plausible alternative to affect the return value of unserialize from
the __wakeup perspective other than throwing an exception. Looking at what @laruence has done in bug
#64354 I think we can throw an exception in __wakeup and __set_state and integrate
DATE_CHECK_INITIALIZED wherever it's missing. This way it won't delete the invalid date
object from the scope, but that object will respond only with false on each method. Here's the
slightly modified snippet from @tstarling


<?php

$s2 = 'O:3:"Foo":3:{s:4:"date";s:20:"10007-06-07
03:51:49";s:13:"timezone_type";i:3;s:8:"timezone";s:3:"UTC";}';

global $foo;

class Foo extends DateTime {
    function __construct() {
        global $foo;
        $foo = $this;
        parent::__construct();
    }
    function __wakeup() {
        global $foo;
        $foo = $this;
        parent::__wakeup();
    }
}

try {
        new Foo("10007-06-07 03:51:49");
} catch ( Exception $e ) {}
var_dump( $foo );

try {
    unserialize( $s2 );
} catch ( Exception $e ) {}
var_dump( $foo );

Either in both cases after normal construct or after unserialize user will end up with an invalid
$foo object. So there is no BC breach as __construct() already throws an exception, making
__wakeup() do the same and checking dateobj->time != NULL in every method after that should be a
sufficient solution.

------------------------------------------------------------------------


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=62852


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=62852&edit=1


Thread (26 messages)

« previous php.bugs (#182598) next »