Skip to content

Commit 6413478

Browse files
committed
ext/curl: fix use-after-free when setting a callback option from its own callback
php_curl_set_callable_handler() frees the previously registered callback with zend_fcc_dtor() before installing the new one. The write/read/header/progress/ etc. callbacks run with ch->in_callback set, so calling curl_setopt() with one of the callable options (CURLOPT_WRITEFUNCTION, CURLOPT_READFUNCTION, ...) from inside such a callback destroys the fcc that is still executing. For a non-closure callback such as [$obj, 'method'] whose object is only referenced by the fcc, this releases $this while its method is still on the stack, resulting in a use-after-free (closures are kept alive by the call frame, method receivers are not). Reject setting a callable option while inside a callback, matching the existing in_callback guards on curl_close() and curl_reset().
1 parent 59e0be1 commit 6413478

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

‎ext/curl/interface.c‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,8 +1611,14 @@ PHP_FUNCTION(curl_copy_handle)
16111611
}
16121612
/* }}} */
16131613

1614-
static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_fcc, zval *callable, bool is_array_config, const char *option_name)
1614+
static bool php_curl_set_callable_handler(php_curl *ch, zend_fcall_info_cache *const handler_fcc, zval *callable, bool is_array_config, const char *option_name)
16151615
{
1616+
/* Replacing a callback would free the fcc that is still executing on the stack. */
1617+
if (ch->in_callback) {
1618+
zend_throw_error(NULL, "%s(): Attempt to set the %s option from a callback", get_active_function_name(), option_name);
1619+
return false;
1620+
}
1621+
16161622
if (ZEND_FCC_INITIALIZED(*handler_fcc)) {
16171623
zend_fcc_dtor(handler_fcc);
16181624
}
@@ -1636,7 +1642,7 @@ static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_f
16361642

16371643
#define HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(curl_ptr, constant_no_function, handler_type, default_method) \
16381644
case constant_no_function##FUNCTION: { \
1639-
bool result = php_curl_set_callable_handler(&curl_ptr->handlers.handler_type->fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \
1645+
bool result = php_curl_set_callable_handler(curl_ptr, &curl_ptr->handlers.handler_type->fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \
16401646
if (!result) { \
16411647
curl_ptr->handlers.handler_type->method = default_method; \
16421648
return FAILURE; \
@@ -1651,7 +1657,7 @@ static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_f
16511657

16521658
#define HANDLE_CURL_OPTION_CALLABLE(curl_ptr, constant_no_function, handler_fcc, c_callback) \
16531659
case constant_no_function##FUNCTION: { \
1654-
bool result = php_curl_set_callable_handler(&curl_ptr->handler_fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \
1660+
bool result = php_curl_set_callable_handler(curl_ptr, &curl_ptr->handler_fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \
16551661
if (!result) { \
16561662
return FAILURE; \
16571663
} \
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-23814 (Setting a callback option from within a curl callback is rejected)
3+
--EXTENSIONS--
4+
curl
5+
--SKIPIF--
6+
<?php
7+
if (!in_array('file', curl_version()['protocols'], true)) {
8+
die('skip file protocol not supported');
9+
}
10+
?>
11+
--FILE--
12+
<?php
13+
14+
$handle = curl_init('file://' . __FILE__);
15+
$callback = static function (CurlHandle $handle, string $data): int {
16+
try {
17+
curl_setopt($handle, CURLOPT_WRITEFUNCTION, static fn($handle, $data) => strlen($data));
18+
} catch (Error $error) {
19+
echo $error->getMessage(), "\n";
20+
}
21+
22+
try {
23+
curl_setopt_array($handle, [CURLOPT_WRITEFUNCTION => null]);
24+
} catch (Error $error) {
25+
echo $error->getMessage(), "\n";
26+
}
27+
28+
return strlen($data);
29+
};
30+
curl_setopt($handle, CURLOPT_WRITEFUNCTION, $callback);
31+
var_dump(curl_exec($handle));
32+
var_dump(curl_setopt($handle, CURLOPT_WRITEFUNCTION, null));
33+
?>
34+
--EXPECT--
35+
curl_setopt(): Attempt to set the CURLOPT_WRITEFUNCTION option from a callback
36+
curl_setopt_array(): Attempt to set the CURLOPT_WRITEFUNCTION option from a callback
37+
bool(true)
38+
bool(true)

0 commit comments

Comments
 (0)