ext/curl: Fix curl_reset() and CURLOPT_POSTFIELDS keeping old request data

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

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,13 +2978,19 @@ 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);
 		ZVAL_UNDEF(&ch->handlers.write_header->stream);
 	}
 	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);
 }
 /* }}} */

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--
+<?php
+include 'server.inc';
+$host = curl_cli_server_start();
+
+foreach ([
+    'CURLOPT_HTTPGET' => [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) ""

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--
+<?php
+include 'server.inc';
+$host = curl_cli_server_start();
+
+$handle = curl_init("{$host}/get.inc?test=input");
+curl_setopt($handle, CURLOPT_RETURNTRANSFER, true);
+curl_setopt($handle, CURLOPT_POSTFIELDS, ['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"

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--
+<?php
+class Marker
+{
+    public function __construct(private string $name) {}
+
+    public function __invoke(...$args): int
+    {
+        return 0;
+    }
+
+    public function __destruct()
+    {
+        echo $this->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)