ext/curl: Guard curl_reset(), copy and clone of a denied pushed handle

NEWS

@@ -10,6 +10,12 @@ PHP                                                                        NEWS
   . Fix GH-21999: GC inconsistency with lazy object, var_dump(), and object
     comparison. (Arnaud)
 
+- Curl:
+  . Fixed a leaked CurlHandle and clone accounting when a CURLMOPT_PUSHFUNCTION
+    callback denies the push or throws. (Ilia Alshanetsky)
+  . Fixed crash in curl_reset(), curl_copy_handle() and clone on a pushed
+    handle kept after CURLMOPT_PUSHFUNCTION denied it. (Ilia Alshanetsky)
+
 - DOM:
   . Fixed use-after-free when re-constructing a DOMXPath whose php:function
     registrations are freed while still reachable from the cycle collector.

ext/curl/interface.c

@@ -443,7 +443,7 @@ static zend_object *curl_clone_obj(zend_object *object) {
 	clone_ch = curl_from_obj(clone_object);
 
 	ch = curl_from_obj(object);
-	cp = curl_easy_duphandle(ch->cp);
+	cp = ch->cp ? curl_easy_duphandle(ch->cp) : NULL;
 	if (!cp) {
 		zend_throw_exception(NULL, "Failed to clone CurlHandle", 0);
 		return &clone_ch->std;
@@ -1613,7 +1613,7 @@ PHP_FUNCTION(curl_copy_handle)
 
 	ch = Z_CURL_P(zid);
 
-	cp = curl_easy_duphandle(ch->cp);
+	cp = ch->cp ? curl_easy_duphandle(ch->cp) : NULL;
 	if (!cp) {
 		php_error_docref(NULL, E_WARNING, "Cannot duplicate cURL handle");
 		RETURN_FALSE;
@@ -2834,7 +2834,7 @@ static void curl_free_obj(zend_object *object)
 	fprintf(stderr, "DTOR CALLED, ch = %x\n", ch);
 #endif
 
-	if (!ch->cp) {
+	if (!ch->clone) {
 		/* Can happen if constructor throws. */
 		zend_object_std_dtor(&ch->std);
 		return;
@@ -3007,7 +3007,9 @@ PHP_FUNCTION(curl_reset)
 		RETURN_THROWS();
 	}
 
-	curl_easy_reset(ch->cp);
+	if (ch->cp) {
+		curl_easy_reset(ch->cp);
+	}
 	_php_curl_reset_handlers(ch);
 	_php_curl_set_default_options(ch);
 }

ext/curl/multi.c

@@ -409,16 +409,15 @@ static int _php_server_push_callback(CURL *parent_ch, CURL *easy, size_t num_hea
 	zend_call_known_fcc(&mh->handlers.server_push, &retval, /* param_count */ 3, call_args, /* named_params */ NULL);
 	zval_ptr_dtor_nogc(&headers);
 
-	if (!Z_ISUNDEF(retval)) {
-		if (CURL_PUSH_DENY != php_curl_get_long(&retval)) {
-		    rval = CURL_PUSH_OK;
-			zend_llist_add_element(&mh->easyh, &pz_ch);
-		} else {
-			/* libcurl will free this easy handle, avoid double free */
-			ch->cp = NULL;
-		}
+	if (!Z_ISUNDEF(retval) && CURL_PUSH_DENY != php_curl_get_long(&retval)) {
+		rval = CURL_PUSH_OK;
+		zend_llist_add_element(&mh->easyh, &pz_ch);
+		return rval;
 	}
 
+	ch->cp = NULL;
+	zval_ptr_dtor(&pz_ch);
+
 	return rval;
 }
 /* }}} */

ext/curl/tests/curl_pushfunction_deny_kept_handle.phpt

@@ -0,0 +1,57 @@
+--TEST--
+CURLMOPT_PUSHFUNCTION denied handle kept by the callback can be reset, copied and cloned
+--EXTENSIONS--
+curl
+--SKIPIF--
+<?php
+include 'skipif-nocaddy.inc';
+
+$curl_version = curl_version();
+if ($curl_version['version_number'] < 0x080100) {
+    exit("skip: test may crash with curl < 8.1.0");
+}
+?>
+--FILE--
+<?php
+$kept = null;
+$callback = function ($parent_ch, $pushed_ch, array $headers) use (&$kept) {
+    $kept = $pushed_ch;
+    return CURL_PUSH_DENY;
+};
+
+$mh = curl_multi_init();
+curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
+curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, $callback);
+
+$ch = curl_init();
+curl_setopt($ch, CURLOPT_URL, "https://localhost/serverpush");
+curl_setopt($ch, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
+curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+curl_multi_add_handle($mh, $ch);
+
+$done = false;
+do {
+    curl_multi_exec($mh, $active);
+    do {
+        $info = curl_multi_info_read($mh);
+        if (false !== $info && $info['msg'] == CURLMSG_DONE) {
+            curl_multi_remove_handle($mh, $info['handle']);
+            $done = true;
+        }
+    } while ($info);
+} while (!$done);
+
+curl_multi_close($mh);
+
+curl_reset($kept);
+var_dump(curl_copy_handle($kept));
+try {
+    clone $kept;
+} catch (Exception $e) {
+    echo $e::class, ": ", $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECTF--
+Warning: curl_copy_handle(): Cannot duplicate cURL handle in %s on line %d
+bool(false)
+Exception: Failed to clone CurlHandle

ext/curl/tests/curl_pushfunction_deny_leak.phpt

@@ -0,0 +1,80 @@
+--TEST--
+CURLMOPT_PUSHFUNCTION deny must not leak the pushed handle
+--EXTENSIONS--
+curl
+--SKIPIF--
+<?php
+include 'skipif-nocaddy.inc';
+
+$curl_version = curl_version();
+if ($curl_version['version_number'] < 0x080100) {
+    exit("skip: test may crash with curl < 8.1.0");
+}
+?>
+--FILE--
+<?php
+$push_count = 0;
+
+function do_request(): void {
+    $callback = function ($parent_ch, $pushed_ch, array $headers) {
+        $GLOBALS['push_count']++;
+        return CURL_PUSH_DENY;
+    };
+
+    $mh = curl_multi_init();
+    curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
+    curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, $callback);
+
+    $ch = curl_init();
+    curl_setopt($ch, CURLOPT_URL, "https://localhost/serverpush");
+    curl_setopt($ch, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
+    curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+    curl_multi_add_handle($mh, $ch);
+
+    $done = false;
+    do {
+        curl_multi_exec($mh, $active);
+
+        do {
+            $info = curl_multi_info_read($mh);
+            if (false !== $info && $info['msg'] == CURLMSG_DONE) {
+                $handle = $info['handle'];
+                if ($handle !== null) {
+                    curl_multi_remove_handle($mh, $handle);
+                    curl_close($handle);
+                    $done = true;
+                }
+            }
+        } while ($info);
+    } while (!$done);
+
+    curl_multi_close($mh);
+}
+
+$iterations = 200;
+$bytes_per_iteration_limit = 512;
+
+do_request();
+gc_collect_cycles();
+
+$before = memory_get_usage();
+for ($i = 0; $i < $iterations; $i++) {
+    do_request();
+}
+gc_collect_cycles();
+$growth = memory_get_usage() - $before;
+
+if ($push_count < $iterations) {
+    printf("too few pushes handled: %d\n", $push_count);
+} else {
+    echo "all pushes denied\n";
+}
+if ($growth > $iterations * $bytes_per_iteration_limit) {
+    printf("leaked %d bytes (%d bytes/iteration)\n", $growth, (int) ($growth / $iterations));
+} else {
+    echo "no leak detected\n";
+}
+?>
+--EXPECT--
+all pushes denied
+no leak detected

ext/curl/tests/curl_pushfunction_deny_retain.phpt

@@ -0,0 +1,57 @@
+--TEST--
+CURLMOPT_PUSHFUNCTION deny with the pushed handle retained by the callback
+--EXTENSIONS--
+curl
+--SKIPIF--
+<?php
+include 'skipif-nocaddy.inc';
+
+$curl_version = curl_version();
+if ($curl_version['version_number'] < 0x080100) {
+    exit("skip: test may crash with curl < 8.1.0");
+}
+?>
+--FILE--
+<?php
+$kept = [];
+$callback = function ($parent_ch, $pushed_ch, array $headers) use (&$kept) {
+    $kept[] = $pushed_ch;
+    return CURL_PUSH_DENY;
+};
+
+$mh = curl_multi_init();
+curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
+curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, $callback);
+
+$ch = curl_init();
+curl_setopt($ch, CURLOPT_URL, "https://localhost/serverpush");
+curl_setopt($ch, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
+curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+curl_multi_add_handle($mh, $ch);
+
+$done = false;
+do {
+    curl_multi_exec($mh, $active);
+    do {
+        $info = curl_multi_info_read($mh);
+        if (false !== $info && $info['msg'] == CURLMSG_DONE) {
+            curl_multi_remove_handle($mh, $info['handle']);
+            $done = true;
+        }
+    } while ($info);
+} while (!$done);
+
+curl_multi_close($mh);
+unset($ch, $info);
+gc_collect_cycles();
+
+var_dump(count($kept));
+var_dump(curl_setopt($kept[0], CURLOPT_WRITEFUNCTION, fn($h, $data) => strlen($data)));
+unset($kept);
+gc_collect_cycles();
+echo "Done\n";
+?>
+--EXPECT--
+int(1)
+bool(true)
+Done