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.

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 07ab456d1a97ecf4a5c5c2464c3b2340f04c4d6b..cc31a81a1ca5837edde0dda0d25b0cb130c7f087 100644 (file)
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -825,6 +825,10 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
 
                                if (zend_fcall_info_init(arg, 0, fci, fcc, NULL, &is_callable_error) == SUCCESS) {
                                        ZEND_ASSERT(!is_callable_error);
+                                       /* Release call trampolines: The function may not get called, in which case
+                                        * the trampoline will leak. Force it to be refetched during
+                                        * zend_call_function instead. */
+                                       zend_release_fcall_info_cache(fcc);
                                        break;
                                }
 
@@ -2979,8 +2983,8 @@ ZEND_API void zend_release_fcall_info_cache(zend_fcall_info_cache *fcc) {
                        zend_string_release_ex(fcc->function_handler->common.function_name, 0);
                }
                zend_free_trampoline(fcc->function_handler);
+               fcc->function_handler = NULL;
        }
-       fcc->function_handler = NULL;
 }
 
 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) /* {{{ */

diff --git a/Zend/zend_API.h b/Zend/zend_API.h
index a81a43f2314de40021c96ebc136ad4bdac21f743..72cd3856aeae6892705832881580c50b9475708c 100644 (file)
--- a/Zend/zend_API.h
+++ b/Zend/zend_API.h
@@ -2004,6 +2004,10 @@ static zend_always_inline bool zend_parse_arg_func(zval *arg, zend_fcall_info *d
        } else if (UNEXPECTED(zend_fcall_info_init(arg, 0, dest_fci, dest_fcc, NULL, error) != SUCCESS)) {
                return 0;
        }
+       /* Release call trampolines: The function may not get called, in which case
+        * the trampoline will leak. Force it to be refetched during
+        * zend_call_function instead. */
+       zend_release_fcall_info_cache(dest_fcc);
        return 1;
 }
 

diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c
index 906f0666edcff63274669952e686ef6e9a543a42..c3a2a1b63f33199cb01facba770e375ac78a1b4c 100644 (file)
--- a/Zend/zend_builtin_functions.c
+++ b/Zend/zend_builtin_functions.c
@@ -1198,7 +1198,6 @@ ZEND_FUNCTION(set_error_handler)
 
        ZVAL_COPY(&EG(user_error_handler), &(fci.function_name));
        EG(user_error_handler_error_reporting) = (int)error_type;
-       zend_release_fcall_info_cache(&fcc);
 }
 /* }}} */
 
@@ -1254,7 +1253,6 @@ ZEND_FUNCTION(set_exception_handler)
        }
 
        ZVAL_COPY(&EG(user_exception_handler), &(fci.function_name));
-       zend_release_fcall_info_cache(&fcc);
 }
 /* }}} */
 

diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c
index d92f774db46328f7541396461e0f9b819571b452..09c876b45f7b46c2dca6e9c9b0b5b5602c6b9622 100644 (file)
--- a/ext/spl/php_spl.c
+++ b/ext/spl/php_spl.c
@@ -519,6 +519,13 @@ PHP_FUNCTION(spl_autoload_register)
 
        /* If first arg is not null */
        if (ZEND_FCI_INITIALIZED(fci)) {
+               if (!fcc.function_handler) {
+                       /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
+                        * with it outselves. It is important that it is not refetched on every call,
+                        * because calls may occur from different scopes. */
+                       zend_is_callable_ex(&fci.function_name, NULL, 0, NULL, &fcc, NULL);
+               }
+
                if (fcc.function_handler->type == ZEND_INTERNAL_FUNCTION &&
                        fcc.function_handler->internal_function.handler == zif_spl_autoload_call) {
                        zend_argument_value_error(1, "must not be the spl_autoload_call() function");
@@ -566,7 +573,7 @@ PHP_FUNCTION(spl_autoload_unregister)
                RETURN_THROWS();
        }
 
-       if (zend_string_equals_literal(
+       if (fcc.function_handler && zend_string_equals_literal(
                        fcc.function_handler->common.function_name, "spl_autoload_call")) {
                /* Don't destroy the hash table, as we might be iterating over it right now. */
                zend_hash_clean(SPL_G(autoload_functions));

diff --git a/ext/standard/array.c b/ext/standard/array.c
index 82b34673fffb878cfcac02b57225c4e678828f2c..b8996f8a342885a7568b3f1fe075951e89c41316 100644 (file)
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -992,7 +992,6 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */
        BG(user_compare_fci_cache) = empty_fcall_info_cache; \
 
 #define PHP_ARRAY_CMP_FUNC_RESTORE() \
-       zend_release_fcall_info_cache(&BG(user_compare_fci_cache)); \
        BG(user_compare_fci) = old_user_compare_fci; \
        BG(user_compare_fci_cache) = old_user_compare_fci_cache; \
 
@@ -1515,7 +1514,6 @@ PHP_FUNCTION(array_walk)
        );
 
        php_array_walk(array, userdata, 0);
-       zend_release_fcall_info_cache(&BG(array_walk_fci_cache));
        BG(array_walk_fci) = orig_array_walk_fci;
        BG(array_walk_fci_cache) = orig_array_walk_fci_cache;
        RETURN_TRUE;
@@ -1545,7 +1543,6 @@ PHP_FUNCTION(array_walk_recursive)
        );
 
        php_array_walk(array, userdata, 1);
-       zend_release_fcall_info_cache(&BG(array_walk_fci_cache));
        BG(array_walk_fci) = orig_array_walk_fci;
        BG(array_walk_fci_cache) = orig_array_walk_fci_cache;
        RETURN_TRUE;
@@ -5951,7 +5948,6 @@ PHP_FUNCTION(array_reduce)
        htbl = Z_ARRVAL_P(input);
 
        if (zend_hash_num_elements(htbl) == 0) {
-               zend_release_fcall_info_cache(&fci_cache);
                return;
        }
 
@@ -5973,8 +5969,6 @@ PHP_FUNCTION(array_reduce)
                        RETURN_NULL();
                }
        } ZEND_HASH_FOREACH_END();
-
-       zend_release_fcall_info_cache(&fci_cache);
 }
 /* }}} */
 
@@ -6002,7 +5996,6 @@ PHP_FUNCTION(array_filter)
 
        if (zend_hash_num_elements(Z_ARRVAL_P(array)) == 0) {
                RETVAL_EMPTY_ARRAY();
-               zend_release_fcall_info_cache(&fci_cache);
                return;
        }
        array_init(return_value);
@@ -6064,8 +6057,6 @@ PHP_FUNCTION(array_filter)
                }
                zval_add_ref(operand);
        } ZEND_HASH_FOREACH_END();
-
-       zend_release_fcall_info_cache(&fci_cache);
 }
 /* }}} */
 
@@ -6092,7 +6083,6 @@ PHP_FUNCTION(array_map)
                int ret;
 
                if (Z_TYPE(arrays[0]) != IS_ARRAY) {
-                       zend_release_fcall_info_cache(&fci_cache);
                        zend_argument_type_error(2, "must be of type array, %s given", zend_zval_type_name(&arrays[0]));
                        RETURN_THROWS();
                }
@@ -6101,7 +6091,6 @@ PHP_FUNCTION(array_map)
                /* Short-circuit: if no callback and only one array, just return it. */
                if (!ZEND_FCI_INITIALIZED(fci) || !maxlen) {
                        ZVAL_COPY(return_value, &arrays[0]);
-                       zend_release_fcall_info_cache(&fci_cache);
                        return;
                }
 
@@ -6126,8 +6115,6 @@ PHP_FUNCTION(array_map)
                                zend_hash_index_add_new(Z_ARRVAL_P(return_value), num_key, &result);
                        }
                } ZEND_HASH_FOREACH_END();
-
-               zend_release_fcall_info_cache(&fci_cache);
        } else {
                uint32_t *array_pos = (HashPosition *)ecalloc(n_arrays, sizeof(HashPosition));
 
@@ -6219,7 +6206,6 @@ PHP_FUNCTION(array_map)
                        }
 
                        efree(params);
-                       zend_release_fcall_info_cache(&fci_cache);
                }
                efree(array_pos);
        }

diff --git a/ext/standard/tests/array/bug74345.phpt b/ext/standard/tests/array/bug74345.phpt
index 5b2bdba95f612d08812dee397f39da6771f96ad7..0916e17a54e1a4a599bf4cf70844dc89ad0a79bd 100644 (file)
--- a/ext/standard/tests/array/bug74345.phpt
+++ b/ext/standard/tests/array/bug74345.phpt
@@ -18,6 +18,11 @@ try {
 } catch (Error $e) {
     echo $e->getMessage(), "\n";
 }
+try {
+    array_map($cb, null, null);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
 array_filter([], $cb);
 array_reduce([], $cb);
 
@@ -26,8 +31,16 @@ array_walk($array, $cb);
 array_walk_recursive($array, $cb);
 usort($array, $cb);
 
+try {
+    preg_replace_callback('/./', $cb, new stdClass);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
 ?>
 ===DONE===
 --EXPECT--
 array_map(): Argument #2 ($array1) must be of type array, null given
+array_map(): Argument #2 ($array1) must be of type array, null given
+preg_replace_callback(): Argument #3 ($subject) must be of type string|array, stdClass given
 ===DONE===