Skip to content

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

Closed
iliaal wants to merge 2 commits into
php:PHP-8.4from
iliaal:fix/curl-push-kept-null-handle
Closed

iliaal wants to merge 2 commits into
php:PHP-8.4from
iliaal:fix/curl-push-kept-null-handle

Conversation

@iliaal

@iliaal iliaal commented Oct 14, 2026

Copy link
Copy Markdown
Member

Depends on #23850 and is stacked on its branch, so the first commit here is that PR's; only the second commit is new. When a CURLMOPT_PUSHFUNCTION callback keeps the pushed handle and denies the push, libcurl frees the easy handle and the CurlHandle is left with a NULL ch->cp. curl_reset(), curl_copy_handle() and clone pass that NULL to curl_easy_reset() and curl_easy_duphandle() and segfault. This predates #23850 (reproduced on 5714009); the other CurlHandle entry points already fail cleanly without an easy handle.

The test depends on the dtor change in #23850: without it the kept handle's handlers leak and a debug build reports it.

Not included: curl_getinfo() with CURLINFO_CERTINFO leaks the array it initialised when curl_easy_getinfo() fails, reachable through the same kept handle. That is a separate fix.

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.
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.
@Sjord

Sjord commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

It seems that curl_pause() / curl_easy_pause() also crashes on a null Curl_easy pointer in some versions.

@bukka
bukka marked this pull request as draft November 7, 2026 15:07
@bukka

bukka commented Nov 7, 2026

Copy link
Copy Markdown
Member

Please create draft if it's not ready for review (e.g. based on another PR).

@iliaal

iliaal commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

Closing: #23850 now gives a denied handle the callback keeps a fresh easy handle, so curl_reset(), curl_copy_handle(), clone and curl_pause() no longer see a NULL one. The test moved there.

@iliaal iliaal closed this Nov 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants