diff --git a/NEWS b/NEWS index 4ed36d63b4f8..1ce16ad82bf8 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,12 @@ PHP NEWS . Fix use-after-free of CURLOPT_STDERR stream on cloned curl handle. (lacatoire) . Fix UAF on clone due to callback data pointer mismatch. (ndossche) + . Fix curl_reset() not releasing the write, header and read callbacks, + CURLOPT_PRIVATE and CURLOPT_POSTFIELDS. (Raj Siva-Rajah) + . Fix curl_copy_handle() restoring file post fields that were replaced. + (Raj Siva-Rajah) + . Fix curl_copy_handle() restoring file post fields after the request method + changes. (Raj Siva-Rajah) - Date: . Fixed bug GH-20503 (Assertion failure with ext/date DateInterval property diff --git a/ext/curl/interface.c b/ext/curl/interface.c index a101e852ee9c..27f0779d7e4d 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1897,6 +1897,15 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue return FAILURE; } error = curl_easy_setopt(ch->cp, option, lval); + /* Copied handles would otherwise rebuild the file fields, overriding these options */ + if (error == CURLE_OK && + (option == CURLOPT_POST || option == CURLOPT_PUT || option == CURLOPT_UPLOAD || + (lval && (option == CURLOPT_HTTPGET || option == CURLOPT_NOBODY)))) { + zval garbage; + ZVAL_COPY_VALUE(&garbage, &ch->postfields); + ZVAL_UNDEF(&ch->postfields); + zval_ptr_dtor(&garbage); + } break; case CURLOPT_SAFE_UPLOAD: if (!zend_is_true(zvalue)) { @@ -2248,6 +2257,13 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue break; case CURLOPT_POSTFIELDS: + { + /* Drop file fields kept from a previous value, which copied handles would otherwise rebuild */ + zval garbage; + ZVAL_COPY_VALUE(&garbage, &ch->postfields); + ZVAL_UNDEF(&ch->postfields); + zval_ptr_dtor(&garbage); + if (Z_TYPE_P(zvalue) == IS_ARRAY) { if (zend_hash_num_elements(HASH_OF(zvalue)) == 0) { /* no need to build the mime structure for empty hashtables; @@ -2266,6 +2282,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue zend_tmp_string_release(tmp_str); } break; + } case CURLOPT_RETURNTRANSFER: if (zend_is_true(zvalue)) { @@ -2961,6 +2978,9 @@ static void _php_curl_reset_handlers(php_curl *ch) } ch->handlers.write->fp = NULL; ch->handlers.write->method = PHP_CURL_STDOUT; + if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { + zend_fcc_dtor(&ch->handlers.write->fcc); + } if (!Z_ISUNDEF(ch->handlers.write_header->stream)) { zval_ptr_dtor(&ch->handlers.write_header->stream); @@ -2968,6 +2988,9 @@ static void _php_curl_reset_handlers(php_curl *ch) } ch->handlers.write_header->fp = NULL; ch->handlers.write_header->method = PHP_CURL_IGNORE; + if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { + zend_fcc_dtor(&ch->handlers.write_header->fcc); + } if (!Z_ISUNDEF(ch->handlers.read->stream)) { zval_ptr_dtor(&ch->handlers.read->stream); @@ -2976,6 +2999,9 @@ static void _php_curl_reset_handlers(php_curl *ch) ch->handlers.read->fp = NULL; ch->handlers.read->res = NULL; ch->handlers.read->method = PHP_CURL_DIRECT; + if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) { + zend_fcc_dtor(&ch->handlers.read->fcc); + } if (!Z_ISUNDEF(ch->handlers.std_err)) { zval_ptr_dtor(&ch->handlers.std_err); @@ -3015,6 +3041,7 @@ PHP_FUNCTION(curl_reset) { zval *zid; php_curl *ch; + zval garbage; ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_OBJECT_OF_CLASS(zid, curl_ce) @@ -3029,6 +3056,15 @@ PHP_FUNCTION(curl_reset) curl_easy_reset(ch->cp); _php_curl_reset_handlers(ch); + + ZVAL_COPY_VALUE(&garbage, &ch->postfields); + ZVAL_UNDEF(&ch->postfields); + zval_ptr_dtor(&garbage); + + ZVAL_COPY_VALUE(&garbage, &ch->private_data); + ZVAL_UNDEF(&ch->private_data); + zval_ptr_dtor(&garbage); + _php_curl_set_default_options(ch); } /* }}} */ diff --git a/ext/curl/tests/curl_copy_handle_file_postfields_method.phpt b/ext/curl/tests/curl_copy_handle_file_postfields_method.phpt new file mode 100644 index 000000000000..bc9526d08873 --- /dev/null +++ b/ext/curl/tests/curl_copy_handle_file_postfields_method.phpt @@ -0,0 +1,37 @@ +--TEST-- +curl_copy_handle() does not restore file post fields after the request method changes +--EXTENSIONS-- +curl +--FILE-- + [CURLOPT_HTTPGET, true], + 'CURLOPT_POST' => [CURLOPT_POST, false], + 'CURLOPT_UPLOAD' => [CURLOPT_UPLOAD, false], + 'CURLOPT_NOBODY' => [CURLOPT_NOBODY, true], +] as $name => [$option, $value]) { + $handle = curl_init("{$host}/get.inc?test=method"); + curl_setopt($handle, CURLOPT_RETURNTRANSFER, true); + curl_setopt($handle, CURLOPT_POSTFIELDS, ['file' => new CURLStringFile('body', 'file.txt')]); + curl_setopt($handle, $option, $value); + + $copy = curl_copy_handle($handle); + var_dump($name, curl_exec($handle), curl_exec($copy)); +} +?> +--EXPECT-- +string(15) "CURLOPT_HTTPGET" +string(3) "GET" +string(3) "GET" +string(12) "CURLOPT_POST" +string(3) "GET" +string(3) "GET" +string(14) "CURLOPT_UPLOAD" +string(3) "GET" +string(3) "GET" +string(14) "CURLOPT_NOBODY" +string(0) "" +string(0) "" diff --git a/ext/curl/tests/curl_copy_handle_replaced_file_postfields.phpt b/ext/curl/tests/curl_copy_handle_replaced_file_postfields.phpt new file mode 100644 index 000000000000..b10f78174554 --- /dev/null +++ b/ext/curl/tests/curl_copy_handle_replaced_file_postfields.phpt @@ -0,0 +1,21 @@ +--TEST-- +curl_copy_handle() does not restore file post fields that were replaced +--EXTENSIONS-- +curl +--FILE-- + new CURLStringFile('old body', 'old.txt')]); +curl_setopt($handle, CURLOPT_POSTFIELDS, 'new body'); + +$copy = curl_copy_handle($handle); +echo curl_exec($handle); +echo curl_exec($copy); +?> +--EXPECT-- +string(8) "new body" +string(8) "new body" diff --git a/ext/curl/tests/curl_reset_releases_options.phpt b/ext/curl/tests/curl_reset_releases_options.phpt new file mode 100644 index 000000000000..2a8fe6506b2e --- /dev/null +++ b/ext/curl/tests/curl_reset_releases_options.phpt @@ -0,0 +1,48 @@ +--TEST-- +curl_reset() releases callbacks, CURLOPT_PRIVATE and file post fields +--EXTENSIONS-- +curl +--FILE-- +name, " destroyed\n"; + } +} + +class File extends CURLStringFile +{ + public function __destruct() + { + echo "post file destroyed\n"; + } +} + +$handle = curl_init(); +curl_setopt($handle, CURLOPT_WRITEFUNCTION, new Marker('write callback')); +curl_setopt($handle, CURLOPT_HEADERFUNCTION, new Marker('header callback')); +curl_setopt($handle, CURLOPT_READFUNCTION, new Marker('read callback')); +curl_setopt($handle, CURLOPT_PRIVATE, new Marker('private value')); +curl_setopt($handle, CURLOPT_POSTFIELDS, ['file' => new File('data', 'file.txt')]); + +curl_reset($handle); +echo "reset\n"; +var_dump(curl_getinfo($handle, CURLINFO_PRIVATE)); +?> +--EXPECT-- +write callback destroyed +header callback destroyed +read callback destroyed +post file destroyed +private value destroyed +reset +bool(false)