From 7783a8d746b873836806d480ec18cfb79640c9c8 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Wed, 3 Dec 2025 15:48:29 +0100 Subject: [PATCH] Fix GH-19520: php_admin_value[extension] in FPM unsafely runs RINIT This redifines start_now param to start_mode in php_dl / php_load_extension to allow selection of start hooks to run. And for php-fpm it uses just starting of module. --- NEWS | 2 + ext/standard/dl.c | 13 ++--- ext/standard/dl.h | 9 +++- main/php_ini.c | 2 +- sapi/fpm/fpm/fpm_php.c | 2 +- .../tests/gh19520-php-value-ext-no-rinit.phpt | 54 +++++++++++++++++++ 6 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 sapi/fpm/tests/gh19520-php-value-ext-no-rinit.phpt diff --git a/NEWS b/NEWS index d6d453231178..4471797db302 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,8 @@ PHP NEWS request paths. (Marcel Hernandez) . Fixed use-after-free of the scoreboard copy when logging invalid status values. (Marcel Hernandez) + . Fixed bug GH-19520 (php_admin_value[extension] in FPM unsafely runs RINIT). + (Jakub Zelenka) . Fixed null pointer dereference in fpm_get_status() when the scoreboard copy fails. (Marcel Hernandez) diff --git a/ext/standard/dl.c b/ext/standard/dl.c index d2b189888a9c..aaee53a26f60 100644 --- a/ext/standard/dl.c +++ b/ext/standard/dl.c @@ -66,7 +66,7 @@ PHPAPI PHP_FUNCTION(dl) zend_rc_debug = false; #endif - php_dl(filename, MODULE_TEMPORARY, return_value, 0); + php_dl(filename, MODULE_TEMPORARY, return_value, PHP_DL_START_NONE); if (Z_TYPE_P(return_value) == IS_TRUE) { EG(full_tables_cleanup) = 1; } @@ -107,7 +107,7 @@ PHPAPI void *php_load_shlib(const char *path, char **errp) /* }}} */ /* {{{ php_load_extension */ -PHPAPI int php_load_extension(const char *filename, int type, int start_now) +PHPAPI int php_load_extension(const char *filename, int type, int start_mode) { void *handle; char *libpath; @@ -238,12 +238,13 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now) module_entry->handle = handle; - if ((type == MODULE_TEMPORARY || start_now) && zend_startup_module_ex(module_entry) == FAILURE) { + if ((type == MODULE_TEMPORARY || start_mode != PHP_DL_START_NONE) && zend_startup_module_ex(module_entry) == FAILURE) { DL_UNLOAD(handle); return FAILURE; } - if ((type == MODULE_TEMPORARY || start_now) && module_entry->request_startup_func) { + if ((type == MODULE_TEMPORARY || (start_mode != PHP_DL_START_NONE && start_mode != PHP_DL_START_MODULE)) + && module_entry->request_startup_func) { if (module_entry->request_startup_func(type, module_entry->module_number) == FAILURE) { php_error_docref(NULL, error_type, "Unable to initialize module '%s'", module_entry->name); DL_UNLOAD(handle); @@ -278,10 +279,10 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now) #endif /* {{{ php_dl */ -PHPAPI void php_dl(const char *file, int type, zval *return_value, int start_now) +PHPAPI void php_dl(const char *file, int type, zval *return_value, int start_mode) { /* Load extension */ - if (php_load_extension(file, type, start_now) == FAILURE) { + if (php_load_extension(file, type, start_mode) == FAILURE) { RETVAL_FALSE; } else { RETVAL_TRUE; diff --git a/ext/standard/dl.h b/ext/standard/dl.h index 58de973bf4b5..410f2150ea12 100644 --- a/ext/standard/dl.h +++ b/ext/standard/dl.h @@ -19,8 +19,13 @@ #ifndef DL_H #define DL_H -PHPAPI int php_load_extension(const char *filename, int type, int start_now); -PHPAPI void php_dl(const char *file, int type, zval *return_value, int start_now); +/* start modes (values 0 and 1 match the former start_now argument) */ +#define PHP_DL_START_NONE 0 /* no startup */ +#define PHP_DL_START_REQUEST 1 /* module and request startup */ +#define PHP_DL_START_MODULE 2 /* module startup only */ + +PHPAPI int php_load_extension(const char *filename, int type, int start_mode); +PHPAPI void php_dl(const char *file, int type, zval *return_value, int start_mode); PHPAPI void *php_load_shlib(const char *path, char **errp); /* dynamic loading functions */ diff --git a/main/php_ini.c b/main/php_ini.c index 67c93d013b3c..bbfa67bc9b86 100644 --- a/main/php_ini.c +++ b/main/php_ini.c @@ -313,7 +313,7 @@ static void php_ini_parser_cb(zval *arg1, zval *arg2, zval *arg3, int callback_t static void php_load_php_extension_cb(void *arg) { #ifdef HAVE_LIBDL - php_load_extension(*((char **) arg), MODULE_PERSISTENT, 0); + php_load_extension(*((char **) arg), MODULE_PERSISTENT, PHP_DL_START_NONE); #endif } /* }}} */ diff --git a/sapi/fpm/fpm/fpm_php.c b/sapi/fpm/fpm/fpm_php.c index 8deb107d0a38..74d2700c2ab6 100644 --- a/sapi/fpm/fpm/fpm_php.c +++ b/sapi/fpm/fpm/fpm_php.c @@ -75,7 +75,7 @@ int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode) /* {{{ */ zend_rc_debug = false; #endif - php_dl(value, MODULE_PERSISTENT, &zv, 1); + php_dl(value, MODULE_PERSISTENT, &zv, PHP_DL_START_MODULE); #if ZEND_RC_DEBUG zend_rc_debug = orig_rc_debug; diff --git a/sapi/fpm/tests/gh19520-php-value-ext-no-rinit.phpt b/sapi/fpm/tests/gh19520-php-value-ext-no-rinit.phpt new file mode 100644 index 000000000000..247ee41bc092 --- /dev/null +++ b/sapi/fpm/tests/gh19520-php-value-ext-no-rinit.phpt @@ -0,0 +1,54 @@ +--TEST-- +FPM: GH-19520 - loading shared ext in FPM config must not run RINIT outside of a request +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->expectLogPattern('/DL TEST MINIT/'); +// RINIT must not run when the extension is loaded during the child init (before any request) +$tester->expectNoLogPattern('/DL TEST RINIT/', checkAllLogs: false, timeoutMicroseconds: 200_000); +$tester->request()->expectBody('bool(true)'); +$tester->expectLogPattern('/DL TEST RINIT/'); +$tester->expectLogPattern('/DL TEST RSHUTDOWN/'); +$tester->request()->expectBody('bool(true)'); +$tester->expectLogPattern('/DL TEST RINIT/'); +$tester->expectLogPattern('/DL TEST RSHUTDOWN/'); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- +