ext/curl: Free the pushed handle when the push callback denies or throws

NEWS

@@ -10,6 +10,10 @@ 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)
+
 - 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/curl_private.h

@@ -138,6 +138,7 @@ void _php_curl_cleanup_handle(php_curl *);
 void _php_curl_multi_cleanup_list(void *data);
 void _php_curl_verify_handlers(php_curl *ch, bool reporterror);
 void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source);
+void _php_curl_free_instance(php_curl *ch);
 
 /* Consumes `zv` */
 zend_long php_curl_get_long(zval *zv);

ext/curl/interface.c

@@ -2855,6 +2855,13 @@ static void curl_free_obj(zend_object *object)
 		efree(ch->clone);
 	}
 
+	_php_curl_free_instance(ch);
+
+	zend_object_std_dtor(&ch->std);
+}
+
+void _php_curl_free_instance(php_curl *ch)
+{
 	smart_str_free(&ch->handlers.write->buf);
 	if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) {
 		zend_fcc_dtor(&ch->handlers.write->fcc);
@@ -2907,8 +2914,6 @@ static void curl_free_obj(zend_object *object)
 	if (ch->share) {
 		OBJ_RELEASE(&ch->share->std);
 	}
-
-	zend_object_std_dtor(&ch->std);
 }
 /* }}} */
 

ext/curl/multi.c

@@ -409,16 +409,17 @@ 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;
+	--(*ch->clone);
+	_php_curl_free_instance(ch);
+	zval_ptr_dtor(&pz_ch);
+
 	return rval;
 }
 /* }}} */

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