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

 ID:                 62852
 Updated by:         a...@php.net
 Reported by:        kasper at webmasteren dot eu
 Summary:            Unserialize Invalid Date causes crash
 Status:             Re-Opened
 Type:               Bug
 Package:            Reproducible crash
 Operating System:   windows, linux
 PHP Version:        Irrelevant
 Assigned To:        laruence
 Block user comment: N
 Private report:     N

 New Comment:

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.


Previous Comments:
------------------------------------------------------------------------
[2013-03-04 12:41:15] a...@php.net

Here's corresponding BT on windows:

 php5.dll!fetch_timezone_offset(timelib_tzinfo * tz, __int64 ts, __int64 * 
transition_time) Line 341C
 php5.dll!timelib_get_time_zone_info(__int64 ts, timelib_tzinfo * tz) Line 415C
 php5.dll!date_format(char * format, int format_len, timelib_time * t, int 
localtime) Line 1046C
 php5.dll!date_object_get_properties(_zval_struct * object) Line 2117C
 php5.dll!php_var_dump(_zval_struct * * struc, int level) Line 129C
 php5.dll!zif_var_dump(int ht, _zval_struct * return_value, _zval_struct * * 
return_value_ptr, _zval_struct * this_ptr, int return_value_used) Line 181C
 php5.dll!zend_do_fcall_common_helper_SPEC(_zend_execute_data * execute_data) 
Line 320C
 php5.dll!ZEND_DO_FCALL_SPEC_CONST_HANDLER(_zend_execute_data * execute_data) 
Line 1629C
 php5.dll!execute(_zend_op_array * op_array) Line 107C
 php5.dll!zend_execute_scripts(int type, _zval_struct * * retval, int 
file_count, ...) Line 1259C
 php5.dll!php_execute_script(_zend_file_handle * primary_file) Line 2316C
 php.exe!main(int argc, char * * argv) Line 1190C
 php.exe!__tmainCRTStartup() Line 586C
 kernel32.dll!@BaseThreadInitThunk@12()Unknown
 ntdll.dll!___RtlUserThreadStart@8()Unknown
 ntdll.dll!__RtlUserThreadStart@8()Unknown

------------------------------------------------------------------------
[2012-09-16 03:53:29] larue...@php.net

@reeze 
first:  it's not about why he want to do this, like:"why do you want to 
unserialize a abnormal data?"

and your new fix, will allow a incomplete date object in $foo, right? 

I don't this this fix is acceptable, the fix should warning about the wrong 
data, 
and result a NULL or FALSE.

------------------------------------------------------------------------
[2012-09-16 02:31:20] re...@php.net

@laruence, What do you think about this, if you have any better solutions
will be much appreciated :)

------------------------------------------------------------------------
[2012-09-16 02:23:42] re...@php.net

@tstarling the partially initialize problem could be fixed by adding
and exception checking. (the attache patch is a fix for this)

As the exception throwing, I think it's not bc break, since before
the fix, it will just crash, fix the crash is not bc break, and
the use could define __walkup method, it may throw exception too,
so I think throw exception won't make unserialize inconsistant.

Just my 2 cents;

------------------------------------------------------------------------
[2012-09-16 02:18:49] re...@php.net

The following patch has been added/updated:

Patch Name: Fix-add-exception-checking
Revision:   1347761929
URL:        
https://bugs.php.net/patch-display.php?bug=62852&patch=Fix-add-exception-checking&revision=1347761929

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


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

Reply via email to