php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #70163 curl_setopt_array() type confusion
Submitted: 2015-07-29 07:15 UTC Modified: 2015-07-30 08:42 UTC
From: andrea dot palazzo at truel dot it Assigned: laruence
Status: Closed Package: cURL related
PHP Version: 7.0.0beta2 OS: Ubuntu x86_64
Private report: No CVE-ID:
 [2015-07-29 07:15 UTC] andrea dot palazzo at truel dot it
Description:
------------
OVERVIEW

In PHP7 curl_setopt_array is prone to a type confusion due to an unsafe use of the Z_RES_P macro.

DETAILS

from ext/curl/interface.c:2794

 if (zend_parse_parameters(ZEND_NUM_ARGS(), "za", &zid, &arr) == FAILURE) {
        return;
    }

   if ((ch = (php_curl*)zend_fetch_resource(Z_RES_P(zid), le_curl_name, le_curl)) == NULL) {
       RETURN_FALSE;
    }

The problem here is that zid is being retrived as a generic zval but then an assumption is made about it being a resource, thus if a numeric value is supplied as first argoment, Z_RES_P would be confused and try to dereference its value as a pointer.

<?php

/*
Crash on invalid read access
*/

curl_setopt_array(1337, array());

?>

php poc.php

Program received signal SIGSEGV, Segmentation fault.
0x000000000098d81f in zend_fetch_resource (res=0x539, 
    resource_type_name=0xb39cf1 "cURL handle", resource_type=10)
    at /home/kingolol/php-7.0.0beta2/Zend/zend_list.c:126
126		if (resource_type == res->type) {
(gdb) x/i $pc
=> 0x98d81f <zend_fetch_resource+23>:	mov    0xc(%rax),%eax
(gdb) p $rax
$109 = 1337

An attacker could exploit such condition by simply crafting a zend_resource value in memory with an arbitrary *ptr which would then be used as a php_curl.
From this point code execution could be achieved in several ways, relying on the *handlers or *to_free fields for example. 
Details about that will follow as soon as further investigations will be conducted.

SOLUTION

For what is my understanding of the code here, just retrieving zid as a resource should do the job. Other way a typecheck should be performed on zid before passing it to Z_RES_P.

Regards,
Andrea



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-07-30 05:59 UTC] laruence@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: laruence
 [2015-07-30 06:04 UTC] stas@php.net
-Type: Security +Type: Bug
 [2015-07-30 07:48 UTC] andrea dot palazzo at truel dot it
Hello,
kudos for the very prompt fix!

I see that you are not considering this one to be security related, but what about the following scenario?

Class XMLHttpDummy {

	public $curlInstance;

	function __call($x, $y) {
		
		...
		curl_setopt_array($this->curlInstance, array("CURLOPT_POSTFIELDS" => xml_encode_request($x, $y), "CURL_BLA" => "whatever"));
		...
		return curl_exec($this->curlInstance);
	
	}

}

If a class of this kind is present in an unserialize context this pretty seems remote code execution to me, and that's not that unlikely to have something like that, is it?

Regards,
Andrea
 [2015-07-30 08:42 UTC] andrea dot palazzo at truel dot it
Just to be clear, I' haven't investigated the remote execution possibility in deep yet, still I suggest you keep this report private until the next release since remotely triggering the bug is possible.
 [2015-08-04 20:54 UTC] ab@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6c0feb0665f3488ffdc2ab33e9e1b8d3a1af93ae
Log: Fixed bug #70163 (curl_setopt_array() type confusion)
 [2016-07-20 11:37 UTC] davey@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6c0feb0665f3488ffdc2ab33e9e1b8d3a1af93ae
Log: Fixed bug #70163 (curl_setopt_array() type confusion)
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC