From 18794dcd7334225c8a0d3e1a6eba15468dd13f1c Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Sun, 25 Oct 2026 00:09:25 +0200 Subject: [PATCH] Fix GH-17733: System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations This properly fixes the entropy computation to take into account function ordering and module versions. This is basically a reintroduction of the previously removed code from GH-5871, but a simpler version. Since this runs once every module startup, the overhead is negligible as it does not influence the request. I chose this simple approach rather than the suggestion to introduce multiple system entropies based on shm/file-cache. This PR also simplifies some other parts, since we can get rid of some workarounds. --- Zend/zend_API.c | 2 - Zend/zend_system_id.c | 15 +++++ ext/opcache/tests/gh17733.phpt | 110 +++++++++++++++++++++++++++++++++ ext/opcache/zend_file_cache.c | 21 +------ 4 files changed, 126 insertions(+), 22 deletions(-) create mode 100644 ext/opcache/tests/gh17733.phpt diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 031a73109fc0..8b7fa4c73b34 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -34,7 +34,6 @@ #include "zend_enum.h" #include "zend_object_handlers.h" #include "zend_observer.h" -#include "zend_system_id.h" #include @@ -3124,7 +3123,6 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend } zend_flf_handlers[zend_flf_count] = flf_info->handler; zend_flf_functions[zend_flf_count] = (zend_function *)reg_function; - zend_add_system_entropy("frameless", ZSTR_VAL(reg_function->function_name), &flf_info->num_args, sizeof(flf_info->num_args)); zend_flf_count++; flf_info++; } diff --git a/Zend/zend_system_id.c b/Zend/zend_system_id.c index 34142d959710..450d497f7c7b 100644 --- a/Zend/zend_system_id.c +++ b/Zend/zend_system_id.c @@ -90,6 +90,21 @@ void zend_finalize_system_id(void) } } + /* Cached scripts can refer to offsets of internal functions. + * Effects of disable_functions and frameless functions must be taken into account. + * Therefore, embed the function names in-order. + * Updating third party extensions may change function signatures, arginfo, offsets, ... + * Therefore, also embed the loaded modules and their versions. */ + ZEND_HASH_MAP_FOREACH_STR_KEY(CG(function_table), zend_string *name) { + PHP_MD5Update(&context, ZSTR_VAL(name), ZSTR_LEN(name) + 1); + } ZEND_HASH_FOREACH_END(); + ZEND_HASH_MAP_FOREACH_PTR(&module_registry, zend_module_entry *mod) { + PHP_MD5Update(&context, mod->name, strlen(mod->name) + 1); + if (EXPECTED(mod->version != NO_VERSION_YET)) { + PHP_MD5Update(&context, mod->version, strlen(mod->version) + 1); + } + } ZEND_HASH_FOREACH_END(); + PHP_MD5Final(digest, &context); php_hash_bin2hex(zend_system_id, digest, sizeof digest); finalized = 1; diff --git a/ext/opcache/tests/gh17733.phpt b/ext/opcache/tests/gh17733.phpt new file mode 100644 index 000000000000..89f604597bd6 --- /dev/null +++ b/ext/opcache/tests/gh17733.phpt @@ -0,0 +1,110 @@ +--TEST-- +GH-17733 (System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations) +--EXTENSIONS-- +opcache +--FILE-- +getMessage(), "\n"; + } + PHP); +} + +function run(string $cache, string $script, string $disable_functions): void { + $ext = ini_get('extension_dir'); + $opcache = PHP_OS_FAMILY === 'Windows' ? 'php_opcache.dll' : 'opcache.so'; + $cmd = [PHP_BINARY, '-n', '-d', 'extension_dir="' . $ext . '"']; + if (file_exists($ext . '/' . $opcache)) { + array_push($cmd, '-d', 'zend_extension=opcache'); + } + array_push($cmd, + '-d', 'opcache.enable_cli=1', + '-d', 'opcache.file_cache="' . $cache . '"', + '-d', 'opcache.file_cache_only=1', + '-d', 'opcache.file_update_protection=0', + '-d', 'opcache.validate_timestamps=0', // Important to tell apart versions + '-d', 'disable_functions=' . $disable_functions, + $script, + ); + $proc = proc_open($cmd, [1 => ['pipe', 'w'], 2 => ['redirect', 1]], $pipes); + echo stream_get_contents($pipes[1]); + proc_close($proc); +} + +function system_ids(string $cache): int { + $pattern = PHP_OS_FAMILY === 'Windows' ? '/*/*' : '/*'; + return count(glob($cache . $pattern, GLOB_ONLYDIR)); +} + +write_script($script, 1); +run($cache, $script, ''); +var_dump(system_ids($cache)); + +write_script($script, 2); +echo "--- Same environment reuses the cache ---\n"; +run($cache, $script, ''); +var_dump(system_ids($cache)); + +echo "--- Shifted function table ---\n"; +run($cache, $script, 'zend_version'); +var_dump(system_ids($cache)); + +echo "--- Disabled function ---\n"; +run($cache, $script, 'str_repeat'); +var_dump(system_ids($cache)); + +write_script($script, 3); +echo "--- Each environment reuses its own cache ---\n"; +run($cache, $script, 'zend_version'); +run($cache, $script, 'str_repeat'); +run($cache, $script, ''); +var_dump(system_ids($cache)); +?> +--CLEAN-- +isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + rmdir($cache); +} +?> +--EXPECT-- +v1 +string(6) "ababab" +int(1) +--- Same environment reuses the cache --- +v1 +string(6) "ababab" +int(1) +--- Shifted function table --- +v2 +string(6) "ababab" +int(2) +--- Disabled function --- +v2 +Error: Call to undefined function str_repeat() +int(3) +--- Each environment reuses its own cache --- +v2 +string(6) "ababab" +v2 +Error: Call to undefined function str_repeat() +v1 +string(6) "ababab" +int(3) diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index febfcdbf18f7..a8463e4252e6 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -532,32 +532,13 @@ static void zend_file_cache_serialize_op_array(zend_op_array *op_arra } if (opline->op2_type == IS_CONST) { SERIALIZE_PTR(opline->op2.zv); - - /* See GH-17733. Reset Z_EXTRA_P(op2) of ZEND_INIT_FCALL, which - * is an offset into the global function table, to avoid calling - * incorrect functions when environment changes. This, and the - * equivalent code below, can be removed once proper system ID - * validation is implemented. */ - if (opline->opcode == ZEND_INIT_FCALL) { - zval *op2 = opline->op2.zv; - UNSERIALIZE_PTR(op2); - Z_EXTRA_P(op2) = 0; - ZEND_VM_SET_OPCODE_HANDLER(opline); - } } #else if (opline->op1_type == IS_CONST) { opline->op1.constant = RT_CONSTANT(opline, opline->op1) - literals; } if (opline->op2_type == IS_CONST) { - zval *op2 = RT_CONSTANT(opline, opline->op2); - opline->op2.constant = op2 - literals; - - /* See GH-17733 and comment above. */ - if (opline->opcode == ZEND_INIT_FCALL) { - Z_EXTRA_P(op2) = 0; - ZEND_VM_SET_OPCODE_HANDLER(opline); - } + opline->op2.constant = RT_CONSTANT(opline, opline->op2) - literals; } #endif #if ZEND_USE_ABS_JMP_ADDR