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"