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/curl_private.h b/ext/curl/curl_private.h index 2635327f422e..585705a2f692 100644 --- a/ext/curl/curl_private.h +++ b/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); diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 336a89a214c4..3cca807b1cfb 100644 --- a/ext/curl/interface.c +++ b/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); } /* }}} */ diff --git a/ext/curl/multi.c b/ext/curl/multi.c index a484373c107b..b74d42f963e6 100644 --- a/ext/curl/multi.c +++ b/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; } /* }}} */ 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