From 114ba3f996158b2c34c7f873605c117c270ca826 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 17:18:47 -0400 Subject: [PATCH] ext/curl: Free or reinitialize the pushed handle when a push is denied When the push callback denies the push or throws, libcurl destroys the pushed easy handle. The CurlHandle created for it was never released, so it leaked along with its share of the parent's free lists, and a callback that kept it was left with a handle that had no easy handle, on which curl_reset(), curl_copy_handle() and clone crashed. Release the wrapper when the callback dropped it; otherwise give it a new easy handle with curl_init() defaults and its own free lists, detached from the parent. A share set from the callback is detached from the doomed easy handle first so it can still be cleaned up, and curl_free_obj() tests ch->clone for the constructor-failure case, since a released wrapper has no easy handle. --- NEWS | 3 + ext/curl/curl_private.h | 1 + ext/curl/interface.c | 81 ++++++++++++++----- ext/curl/multi.c | 21 +++-- .../tests/curl_pushfunction_deny_leak.phpt | 49 +++++++++++ .../tests/curl_pushfunction_deny_retain.phpt | 69 ++++++++++++++++ 6 files changed, 198 insertions(+), 26 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 2d3e9d00d30d..0c24c4688d10 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,9 @@ PHP NEWS - Curl: . Fixed curl_reset() retaining CURLOPT_PRIVATE after reset. (lacatoire) + . Fixed a leaked CurlHandle when a CURLMOPT_PUSHFUNCTION callback denies the + push or throws, and a crash using the denied handle if the callback kept + it. (Ilia Alshanetsky) - MBString: . Fixed stack overflow in mb_check_encoding() and mb_convert_encoding() with diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 2635327f422e..fc71a3eed53b 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -135,6 +135,7 @@ typedef struct _php_curlsh { php_curl *init_curl_handle_into_zval(zval *curl); void init_curl_handle(php_curl *ch); void _php_curl_cleanup_handle(php_curl *); +void _php_curl_reinit_handle(php_curl *ch); 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); diff --git a/ext/curl/interface.c b/ext/curl/interface.c index b8c6ce7dde3e..55d0b95185ca 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1151,9 +1151,35 @@ php_curl *init_curl_handle_into_zval(zval *curl) return ch; } -void init_curl_handle(php_curl *ch) +static void curl_init_to_free(php_curl *ch) { ch->to_free = ecalloc(1, sizeof(struct _php_curl_free)); + zend_llist_init(&ch->to_free->post, sizeof(struct HttpPost *), (llist_dtor_func_t)curl_free_post, 0); + zend_llist_init(&ch->to_free->stream, sizeof(struct mime_data_cb_arg *), (llist_dtor_func_t)curl_free_cb_arg, 0); + + ch->to_free->slist = emalloc(sizeof(HashTable)); + zend_hash_init(ch->to_free->slist, 4, NULL, curl_free_slist, 0); + + ch->clone = emalloc(sizeof(uint32_t)); + *ch->clone = 1; +} + +static void curl_release_to_free(php_curl *ch) +{ + if (--(*ch->clone) == 0) { + zend_llist_clean(&ch->to_free->post); + zend_llist_clean(&ch->to_free->stream); + + zend_hash_destroy(ch->to_free->slist); + efree(ch->to_free->slist); + efree(ch->to_free); + efree(ch->clone); + } +} + +void init_curl_handle(php_curl *ch) +{ + curl_init_to_free(ch); ch->handlers.write = ecalloc(1, sizeof(php_curl_write)); ch->handlers.write_header = ecalloc(1, sizeof(php_curl_write)); ch->handlers.read = ecalloc(1, sizeof(php_curl_read)); @@ -1167,16 +1193,9 @@ void init_curl_handle(php_curl *ch) #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ ch->handlers.sshhostkey = empty_fcall_info_cache; #endif - ch->clone = emalloc(sizeof(uint32_t)); - *ch->clone = 1; memset(&ch->err, 0, sizeof(struct _php_curl_error)); - zend_llist_init(&ch->to_free->post, sizeof(struct HttpPost *), (llist_dtor_func_t)curl_free_post, 0); - zend_llist_init(&ch->to_free->stream, sizeof(struct mime_data_cb_arg *), (llist_dtor_func_t)curl_free_cb_arg, 0); - - ch->to_free->slist = emalloc(sizeof(HashTable)); - zend_hash_init(ch->to_free->slist, 4, NULL, curl_free_slist, 0); ZVAL_UNDEF(&ch->postfields); } @@ -2854,7 +2873,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; @@ -2865,15 +2884,7 @@ static void curl_free_obj(zend_object *object) curl_easy_cleanup(ch->cp); /* cURL destructors should be invoked only by last curl handle */ - if (--(*ch->clone) == 0) { - zend_llist_clean(&ch->to_free->post); - zend_llist_clean(&ch->to_free->stream); - - zend_hash_destroy(ch->to_free->slist); - efree(ch->to_free->slist); - efree(ch->to_free); - efree(ch->clone); - } + curl_release_to_free(ch); smart_str_free(&ch->handlers.write->buf); if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { @@ -3010,6 +3021,40 @@ static void _php_curl_reset_handlers(php_curl *ch) } /* }}} */ +void _php_curl_reinit_handle(php_curl *ch) +{ + ch->cp = curl_easy_init(); + if (!ch->cp) { + return; + } + + _php_curl_reset_handlers(ch); + if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { + zend_fcc_dtor(&ch->handlers.write->fcc); + } + if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { + zend_fcc_dtor(&ch->handlers.write_header->fcc); + } + if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) { + zend_fcc_dtor(&ch->handlers.read->fcc); + } + _php_curl_cleanup_handle(ch); + + zval_ptr_dtor(&ch->postfields); + ZVAL_UNDEF(&ch->postfields); + zval_ptr_dtor(&ch->private_data); + ZVAL_UNDEF(&ch->private_data); + if (ch->share) { + OBJ_RELEASE(&ch->share->std); + ch->share = NULL; + } + + curl_release_to_free(ch); + curl_init_to_free(ch); + + _php_curl_set_default_options(ch); +} + /* {{{ Reset all options of a libcurl session handle */ PHP_FUNCTION(curl_reset) { diff --git a/ext/curl/multi.c b/ext/curl/multi.c index a484373c107b..1c08b77d75f3 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -409,15 +409,20 @@ 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; + } + + if (ch->share) { + curl_easy_setopt(easy, CURLOPT_SHARE, NULL); + } + ch->cp = NULL; + if (Z_REFCOUNT(pz_ch) > 1) { + _php_curl_reinit_handle(ch); } + 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..a182e212c42f --- /dev/null +++ b/ext/curl/tests/curl_pushfunction_deny_leak.phpt @@ -0,0 +1,49 @@ +--TEST-- +CURLMOPT_PUSHFUNCTION deny frees the pushed handle +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +get()); +?> +--EXPECT-- +string(13) "main response" +array(1) { + [0]=> + string(24) ":path:/serverpush/pushed" +} +NULL 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..e18599ac2408 --- /dev/null +++ b/ext/curl/tests/curl_pushfunction_deny_retain.phpt @@ -0,0 +1,69 @@ +--TEST-- +CURLMOPT_PUSHFUNCTION denied handle kept by the callback is a fresh handle +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- + +--EXPECT-- +bool(true) +bool(false) +bool(true) +bool(true) +Caddy is up and runningbool(true) +int(200) +string(23) "Caddy is up and running" +string(23) "Caddy is up and running" +string(23) "Caddy is up and running"