Skip to content

Commit 2e21818

Browse files
committed
Release call trampolines in zpp fcc
When using zpp 'f' or Z_PARAM_FUNC, if the fcc points to a call trampoline release it immediately and force zend_call_function to refetch it. This may require additional callability checks if __call is used, but avoids the need to carefully free fcc values in all internal functions -- in some cases this is not simple, as a type error might be triggered by a later argument in the same zpp call. This fixes oss-fuzz #25390. Closes GH-6073.
1 parent c0d6b05 commit 2e21818

File tree

6 files changed

+30
-18
lines changed

6 files changed

+30
-18
lines changed

Zend/zend_API.c

+5-1
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,10 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
825825

826826
if (zend_fcall_info_init(arg, 0, fci, fcc, NULL, &is_callable_error) == SUCCESS) {
827827
ZEND_ASSERT(!is_callable_error);
828+
/* Release call trampolines: The function may not get called, in which case
829+
* the trampoline will leak. Force it to be refetched during
830+
* zend_call_function instead. */
831+
zend_release_fcall_info_cache(fcc);
828832
break;
829833
}
830834

@@ -2979,8 +2983,8 @@ ZEND_API void zend_release_fcall_info_cache(zend_fcall_info_cache *fcc) {
29792983
zend_string_release_ex(fcc->function_handler->common.function_name, 0);
29802984
}
29812985
zend_free_trampoline(fcc->function_handler);
2986+
fcc->function_handler = NULL;
29822987
}
2983-
fcc->function_handler = NULL;
29842988
}
29852989

29862990
static zend_always_inline bool zend_is_callable_check_func(int check_flags, zval *callable, zend_execute_data *frame, zend_fcall_info_cache *fcc, bool strict_class, char **error) /* {{{ */

Zend/zend_API.h

+4
Original file line numberDiff line numberDiff line change
@@ -2004,6 +2004,10 @@ static zend_always_inline bool zend_parse_arg_func(zval *arg, zend_fcall_info *d
20042004
} else if (UNEXPECTED(zend_fcall_info_init(arg, 0, dest_fci, dest_fcc, NULL, error) != SUCCESS)) {
20052005
return 0;
20062006
}
2007+
/* Release call trampolines: The function may not get called, in which case
2008+
* the trampoline will leak. Force it to be refetched during
2009+
* zend_call_function instead. */
2010+
zend_release_fcall_info_cache(dest_fcc);
20072011
return 1;
20082012
}
20092013

Zend/zend_builtin_functions.c

-2
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,6 @@ ZEND_FUNCTION(set_error_handler)
11981198

11991199
ZVAL_COPY(&EG(user_error_handler), &(fci.function_name));
12001200
EG(user_error_handler_error_reporting) = (int)error_type;
1201-
zend_release_fcall_info_cache(&fcc);
12021201
}
12031202
/* }}} */
12041203

@@ -1254,7 +1253,6 @@ ZEND_FUNCTION(set_exception_handler)
12541253
}
12551254

12561255
ZVAL_COPY(&EG(user_exception_handler), &(fci.function_name));
1257-
zend_release_fcall_info_cache(&fcc);
12581256
}
12591257
/* }}} */
12601258

ext/spl/php_spl.c

+8-1
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,13 @@ PHP_FUNCTION(spl_autoload_register)
519519

520520
/* If first arg is not null */
521521
if (ZEND_FCI_INITIALIZED(fci)) {
522+
if (!fcc.function_handler) {
523+
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
524+
* with it outselves. It is important that it is not refetched on every call,
525+
* because calls may occur from different scopes. */
526+
zend_is_callable_ex(&fci.function_name, NULL, 0, NULL, &fcc, NULL);
527+
}
528+
522529
if (fcc.function_handler->type == ZEND_INTERNAL_FUNCTION &&
523530
fcc.function_handler->internal_function.handler == zif_spl_autoload_call) {
524531
zend_argument_value_error(1, "must not be the spl_autoload_call() function");
@@ -566,7 +573,7 @@ PHP_FUNCTION(spl_autoload_unregister)
566573
RETURN_THROWS();
567574
}
568575

569-
if (zend_string_equals_literal(
576+
if (fcc.function_handler && zend_string_equals_literal(
570577
fcc.function_handler->common.function_name, "spl_autoload_call")) {
571578
/* Don't destroy the hash table, as we might be iterating over it right now. */
572579
zend_hash_clean(SPL_G(autoload_functions));

ext/standard/array.c

-14
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,6 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */
992992
BG(user_compare_fci_cache) = empty_fcall_info_cache; \
993993

994994
#define PHP_ARRAY_CMP_FUNC_RESTORE() \
995-
zend_release_fcall_info_cache(&BG(user_compare_fci_cache)); \
996995
BG(user_compare_fci) = old_user_compare_fci; \
997996
BG(user_compare_fci_cache) = old_user_compare_fci_cache; \
998997

@@ -1515,7 +1514,6 @@ PHP_FUNCTION(array_walk)
15151514
);
15161515

15171516
php_array_walk(array, userdata, 0);
1518-
zend_release_fcall_info_cache(&BG(array_walk_fci_cache));
15191517
BG(array_walk_fci) = orig_array_walk_fci;
15201518
BG(array_walk_fci_cache) = orig_array_walk_fci_cache;
15211519
RETURN_TRUE;
@@ -1545,7 +1543,6 @@ PHP_FUNCTION(array_walk_recursive)
15451543
);
15461544

15471545
php_array_walk(array, userdata, 1);
1548-
zend_release_fcall_info_cache(&BG(array_walk_fci_cache));
15491546
BG(array_walk_fci) = orig_array_walk_fci;
15501547
BG(array_walk_fci_cache) = orig_array_walk_fci_cache;
15511548
RETURN_TRUE;
@@ -5951,7 +5948,6 @@ PHP_FUNCTION(array_reduce)
59515948
htbl = Z_ARRVAL_P(input);
59525949

59535950
if (zend_hash_num_elements(htbl) == 0) {
5954-
zend_release_fcall_info_cache(&fci_cache);
59555951
return;
59565952
}
59575953

@@ -5973,8 +5969,6 @@ PHP_FUNCTION(array_reduce)
59735969
RETURN_NULL();
59745970
}
59755971
} ZEND_HASH_FOREACH_END();
5976-
5977-
zend_release_fcall_info_cache(&fci_cache);
59785972
}
59795973
/* }}} */
59805974

@@ -6002,7 +5996,6 @@ PHP_FUNCTION(array_filter)
60025996

60035997
if (zend_hash_num_elements(Z_ARRVAL_P(array)) == 0) {
60045998
RETVAL_EMPTY_ARRAY();
6005-
zend_release_fcall_info_cache(&fci_cache);
60065999
return;
60076000
}
60086001
array_init(return_value);
@@ -6064,8 +6057,6 @@ PHP_FUNCTION(array_filter)
60646057
}
60656058
zval_add_ref(operand);
60666059
} ZEND_HASH_FOREACH_END();
6067-
6068-
zend_release_fcall_info_cache(&fci_cache);
60696060
}
60706061
/* }}} */
60716062

@@ -6092,7 +6083,6 @@ PHP_FUNCTION(array_map)
60926083
int ret;
60936084

60946085
if (Z_TYPE(arrays[0]) != IS_ARRAY) {
6095-
zend_release_fcall_info_cache(&fci_cache);
60966086
zend_argument_type_error(2, "must be of type array, %s given", zend_zval_type_name(&arrays[0]));
60976087
RETURN_THROWS();
60986088
}
@@ -6101,7 +6091,6 @@ PHP_FUNCTION(array_map)
61016091
/* Short-circuit: if no callback and only one array, just return it. */
61026092
if (!ZEND_FCI_INITIALIZED(fci) || !maxlen) {
61036093
ZVAL_COPY(return_value, &arrays[0]);
6104-
zend_release_fcall_info_cache(&fci_cache);
61056094
return;
61066095
}
61076096

@@ -6126,8 +6115,6 @@ PHP_FUNCTION(array_map)
61266115
zend_hash_index_add_new(Z_ARRVAL_P(return_value), num_key, &result);
61276116
}
61286117
} ZEND_HASH_FOREACH_END();
6129-
6130-
zend_release_fcall_info_cache(&fci_cache);
61316118
} else {
61326119
uint32_t *array_pos = (HashPosition *)ecalloc(n_arrays, sizeof(HashPosition));
61336120

@@ -6219,7 +6206,6 @@ PHP_FUNCTION(array_map)
62196206
}
62206207

62216208
efree(params);
6222-
zend_release_fcall_info_cache(&fci_cache);
62236209
}
62246210
efree(array_pos);
62256211
}

ext/standard/tests/array/bug74345.phpt

+13
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ try {
1818
} catch (Error $e) {
1919
echo $e->getMessage(), "\n";
2020
}
21+
try {
22+
array_map($cb, null, null);
23+
} catch (Error $e) {
24+
echo $e->getMessage(), "\n";
25+
}
2126
array_filter([], $cb);
2227
array_reduce([], $cb);
2328

@@ -26,8 +31,16 @@ array_walk($array, $cb);
2631
array_walk_recursive($array, $cb);
2732
usort($array, $cb);
2833

34+
try {
35+
preg_replace_callback('/./', $cb, new stdClass);
36+
} catch (Error $e) {
37+
echo $e->getMessage(), "\n";
38+
}
39+
2940
?>
3041
===DONE===
3142
--EXPECT--
3243
array_map(): Argument #2 ($array1) must be of type array, null given
44+
array_map(): Argument #2 ($array1) must be of type array, null given
45+
preg_replace_callback(): Argument #3 ($subject) must be of type string|array, stdClass given
3346
===DONE===

0 commit comments

Comments
 (0)