From ec8ddd886f8c7a0b7407c0d72c77fd43bcdc4e3b Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 17:18:47 -0400 Subject: [PATCH 1/2] ext/curl: Free the pushed handle when the push callback denies or throws When the callback denies the push, or throws and leaves the return value undefined, libcurl destroys the easy handle itself. The wrapper kept a stale ch->cp after a throw, and in both cases never released the CurlHandle it had just created, so each denied push leaked one wrapper and permanently bumped the parent's clone counter. Clear ch->cp and drop the callback's reference instead. curl_free_obj() now keys its early return on ch->clone, which is unset only when the constructor failed, so a handle whose easy handle libcurl already freed still releases its handlers when its last reference goes, including when the callback kept it. --- NEWS | 4 + ext/curl/interface.c | 2 +- ext/curl/multi.c | 15 ++-- .../tests/curl_pushfunction_deny_leak.phpt | 80 +++++++++++++++++++ .../tests/curl_pushfunction_deny_retain.phpt | 57 +++++++++++++ 5 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 ext/curl/tests/curl_pushfunction_deny_leak.phpt create mode 100644 ext/curl/tests/curl_pushfunction_deny_retain.phpt diff --git a/NEWS b/NEWS index e7f45c810b23..7d3e5ea0ecba 100644 --- a/NEWS +++ b/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. diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 336a89a214c4..d51099e92ecb 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -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; diff --git a/ext/curl/multi.c b/ext/curl/multi.c index a484373c107b..cdcfdc2418b1 100644 --- a/ext/curl/multi.c +++ b/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; } /* }}} */ diff --git a/ext/curl/tests/curl_pushfunction_deny_leak.phpt b/ext/curl/tests/curl_pushfunction_deny_leak.phpt new file mode 100644 index 000000000000..f10d158c871d --- /dev/null +++ b/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-- + +--FILE-- + $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 diff --git a/ext/curl/tests/curl_pushfunction_deny_retain.phpt b/ext/curl/tests/curl_pushfunction_deny_retain.phpt new file mode 100644 index 000000000000..4f33f2ca3911 --- /dev/null +++ b/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-- + +--FILE-- + strlen($data))); +unset($kept); +gc_collect_cycles(); +echo "Done\n"; +?> +--EXPECT-- +int(1) +bool(true) +Done From a422e83d83ff42265139c9aa100849ec9f86275a Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Tue, 13 Oct 2026 07:59:18 -0400 Subject: [PATCH 2/2] ext/curl: Guard curl_reset(), copy and clone of a denied pushed handle When a CURLMOPT_PUSHFUNCTION callback keeps the pushed CurlHandle and denies the push, libcurl frees the easy handle and the wrapper is left with ch->cp set to NULL. curl_easy_reset() and curl_easy_duphandle() do not accept a NULL handle, so curl_reset(), curl_copy_handle() and clone on the kept handle crash. Skip the libcurl reset and take the existing duplication failure exits when there is no easy handle. --- NEWS | 2 + ext/curl/interface.c | 8 ++- .../curl_pushfunction_deny_kept_handle.phpt | 57 +++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 ext/curl/tests/curl_pushfunction_deny_kept_handle.phpt diff --git a/NEWS b/NEWS index 7d3e5ea0ecba..df806c00eec6 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ PHP NEWS - 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 diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d51099e92ecb..7d30d0fbbb53 100644 --- a/ext/curl/interface.c +++ b/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; @@ -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); } diff --git a/ext/curl/tests/curl_pushfunction_deny_kept_handle.phpt b/ext/curl/tests/curl_pushfunction_deny_kept_handle.phpt new file mode 100644 index 000000000000..5079419f809c --- /dev/null +++ b/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-- + +--FILE-- +getMessage(), PHP_EOL; +} +?> +--EXPECTF-- +Warning: curl_copy_handle(): Cannot duplicate cURL handle in %s on line %d +bool(false) +Exception: Failed to clone CurlHandle