php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Sec Bug #73091 Unserializing DateInterval object may lead to __toString invocation
Submitted: 2016-09-15 15:20 UTC Modified: 2016-10-11 23:51 UTC
From: yannayl at checkpoint dot com Assigned: stas (profile)
Status: Closed Package: *General Issues
PHP Version: 7.0.11 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: yannayl at checkpoint dot com
New email:
PHP Version: OS:

 

 [2016-09-15 15:20 UTC] yannayl at checkpoint dot com 
Description:
------------
Description:
------------
Unserializing DateInterval object may lead to __toString invocation of arbitrary objects.
This in turn may trigger a plethora of bugs.

In php_date.c, php_date_interval_initialize_from_hash, "days" and "special_amount" fields are retrieved through PHP_DATE_INTERVAL_READ_PROPERTY_I64() which perform a zval_get_string() that, in case of Object, call its __toString() method (lines: 4152, 4154 commit 01e798fa360bcd89980d1946503a8e0f8a2fd357).


This issue is mentioned in #70121, but not treated.

PoC:
----
<?php
class foo {
    function __toString() {
        var_dump(0);
        return 'may be a bug';
    }
}
    
unserialize('O:12:"DateInterval":1:{s:4:"days";O:3:"foo":0:{}}');
?>

result: dump of 0 printed. no errors:
expected result: exception in unserialize when expecting string and receiving object.


GDB:
----
(gdb) b zif_var_dump 
Breakpoint 1 at 0x7a3dde: file /home/yannayl/sources/php-src/ext/standard/var.c, line 200.
(gdb) r dateint_tostring.php 
Starting program: /home/yannayl/sources/php-src/sapi/cli/php dateint_tostring.php
Breakpoint 1, zif_var_dump (execute_data=0x7ffff44131b0, return_value=0x7fffffff8be0) at /home/yannayl/sources/php-src/ext/standard/var.c:200
200 {
(gdb) bt
#0  zif_var_dump (execute_data=0x7ffff44131b0, return_value=0x7fffffff8be0) at /home/yannayl/sources/php-src/ext/standard/var.c:200
#1  0x00000000009561c2 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER () at /home/yannayl/sources/php-src/Zend/zend_vm_execute.h:628
#2  0x00000000009543f6 in execute_ex (ex=0x7ffff4413150) at /home/yannayl/sources/php-src/Zend/zend_vm_execute.h:429
#3  0x0000000000897207 in zend_call_function (fci=0x7fffffff9070, fci_cache=0x7fffffff9040) at /home/yannayl/sources/php-src/Zend/zend_execute_API.c:825
#4  0x00000000008f7fbb in zend_call_method (object=0x7ffff445b7a0, obj_ce=0x7ffff4403018, fn_proxy=0x7ffff4403148, function_name=0xe926d0 "__tostring", function_name_len=10, retval_ptr=0x7fffffff91d0, 
    param_count=0, arg1=0x0, arg2=0x0) at /home/yannayl/sources/php-src/Zend/zend_interfaces.c:102
#5  0x0000000000936aee in zend_std_cast_object_tostring (readobj=0x7ffff445b7a0, writeobj=0x7fffffff9320, type=6) at /home/yannayl/sources/php-src/Zend/zend_object_handlers.c:1629
#6  0x00000000008a2831 in _zval_get_string_func (op=0x7ffff445b7a0) at /home/yannayl/sources/php-src/Zend/zend_operators.c:864
#7  0x0000000000431d4a in _zval_get_string (op=0x7ffff445b7a0) at /home/yannayl/sources/php-src/Zend/zend_operators.h:273
#8  php_date_interval_initialize_from_hash (return_value=0x7fffffff9550, intobj=0x7fffffff9568, myht=0x7ffff44022d8) at /home/yannayl/sources/php-src/ext/date/php_date.c:4152
#9  0x0000000000432276 in zim_DateInterval___wakeup (execute_data=0x7ffff4413100, return_value=0x7fffffff9aa0) at /home/yannayl/sources/php-src/ext/date/php_date.c:4194
#10 0x00000000008972de in zend_call_function (fci=0x7fffffff9890, fci_cache=0x7fffffff97a0) at /home/yannayl/sources/php-src/Zend/zend_execute_API.c:838
#11 0x000000000089632d in _call_user_function_ex (object=0x7fffffffa150, function_name=0x7fffffff9ab0, retval_ptr=0x7fffffff9aa0, param_count=0, params=0x0, no_separation=1)
    at /home/yannayl/sources/php-src/Zend/zend_execute_API.c:671
#12 0x00000000007cca03 in object_common2 (rval=0x7fffffffa150, p=0x7fffffff9f98, max=0x7ffff4467189 "", var_hash=0x7fffffff9fa0, elements=1) at ext/standard/var_unserializer.re:532
#13 0x00000000007cf09e in php_var_unserialize_internal (rval=0x7fffffffa150, p=0x7fffffff9f98, max=0x7ffff4467189 "", var_hash=0x7fffffff9fa0) at ext/standard/var_unserializer.re:950
#14 0x00000000007ccbe1 in php_var_unserialize (rval=0x7fffffffa150, p=0x7fffffff9f98, max=0x7ffff4467189 "", var_hash=0x7fffffff9fa0) at ext/standard/var_unserializer.re:556
#15 0x00000000007af292 in zif_unserialize (execute_data=0x7ffff44130a0, return_value=0x7fffffffa150) at /home/yannayl/sources/php-src/ext/standard/var.c:1108
#16 0x00000000009561c2 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER () at /home/yannayl/sources/php-src/Zend/zend_vm_execute.h:628
#17 0x00000000009543f6 in execute_ex (ex=0x7ffff4413030) at /home/yannayl/sources/php-src/Zend/zend_vm_execute.h:429
#18 0x0000000000954d6a in zend_execute (op_array=0x7ffff44700e0, return_value=0x0) at /home/yannayl/sources/php-src/Zend/zend_vm_execute.h:474
#19 0x00000000008b705b in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /home/yannayl/sources/php-src/Zend/zend.c:1464
#20 0x00000000007ff9da in php_execute_script (primary_file=0x7fffffffca50) at /home/yannayl/sources/php-src/main/main.c:2537
#21 0x0000000000a590c8 in do_cli (argc=2, argv=0x11d3300) at /home/yannayl/sources/php-src/sapi/cli/php_cli.c:990
#22 0x0000000000a5a4a3 in main (argc=2, argv=0x11d3300) at /home/yannayl/sources/php-src/sapi/cli/php_cli.c:1378

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-09-25 23:44 UTC] stas@php.net
-PHP Version: 7.1.0RC1 +PHP Version: 7.0.11 -Assigned To: +Assigned To: stas
 [2016-09-25 23:44 UTC] stas@php.net 
The fix is in security repo as 1b29e4488e19c89e5b37ecb26acaec443d7f1355 and in https://gist.github.com/8c4125af9766d591072526b25bebce08

please verify
 [2016-09-26 08:44 UTC] yannayl at checkpoint dot com 
I think it's better testing '== IS_LONG' and '== IS_STRING' explicitly.
Testing '<= IS_STRING' is much more open ended than needed and may cause issues.
 [2016-09-26 19:43 UTC] stas@php.net 
Which kind of issues? Could you provide an example of the issue caused by it?
 [2016-09-27 09:19 UTC] yannayl at checkpoint dot com 
I don't know of any concrete issue affecting this version, but I am worried that something similar to #68942 (CVE-2015-0273) might happen again.
IMHO avoiding unnecessary conversions makes the code safer.
 [2016-10-11 23:52 UTC] stas@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1b29e4488e19c89e5b37ecb26acaec443d7f1355
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-11 23:52 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-10-12 14:26 UTC] ab@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1b29e4488e19c89e5b37ecb26acaec443d7f1355
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-14 01:02 UTC] ab@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c4254588a9d3b7067f84dbb84db4afcf646b5b40
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-14 02:23 UTC] ab@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1b29e4488e19c89e5b37ecb26acaec443d7f1355
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-14 02:23 UTC] ab@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c4254588a9d3b7067f84dbb84db4afcf646b5b40
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-17 10:07 UTC] bwoebi@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c4254588a9d3b7067f84dbb84db4afcf646b5b40
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 [2016-10-17 10:07 UTC] bwoebi@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1b29e4488e19c89e5b37ecb26acaec443d7f1355
Log: Fix bug #73091 - Unserializing DateInterval object may lead to __toString invocation
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 11:01:29 2024 UTC