From 368e195c8803155b6e74a7aef1a0f473c200ca3f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 19 Sep 2023 20:18:55 +0200 Subject: [PATCH] Fix GH-12215: Module entry being overwritten causes type errors in ext/dom When we try to load an extension multiple times, we still overwrite the type, module number, and handle. If the module number is used to indicate module boundaries (e.g. in reflection and in dom, see e.g. dom_objects_set_class_ex), then all sorts of errors can happen. In the case of ext/dom, OP's error happens because the following happens: - The property handler is set up incorrectly in dom_objects_set_class_ex() because the wrong module number is specified. The class highest in the hierarchy is DOMNode, so the property handler is incorrectly set to that of DOMNode instead of DOMDocument. - The documentElement property doesn't exist on DOMNode, it only exists on DOMDocument, so it tries to read using zend_std_read_property(). As there is no user property called documentElement, that read operation returns an undef value. However, the type is still checked, resulting in the strange exception. Solve this by changing the API such that the data is only overwritten if it's owned data. --- UPGRADING.INTERNALS | 5 +++++ Zend/zend_API.c | 15 +++++++++------ Zend/zend_API.h | 2 +- Zend/zend_builtin_functions.c | 4 +--- ext/standard/dl.c | 7 +++---- sapi/phpdbg/phpdbg_prompt.c | 8 +++----- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 5b8ccbba021e9..781d56cc1bc49 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -14,6 +14,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES 1. Internal API changes ======================== +* zend_register_module_ex() now takes an additional int module_type argument. + This function will also assign the module number and type, there is no need + to do this at the call site anymore. Writing the handle should happen after + successful registration. + ======================== 2. Build system changes ======================== diff --git a/Zend/zend_API.c b/Zend/zend_API.c index e7160246991d4..776659ca11934 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2446,7 +2446,7 @@ ZEND_API void zend_destroy_modules(void) /* {{{ */ } /* }}} */ -ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) /* {{{ */ +ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module, int module_type) /* {{{ */ { size_t name_len; zend_string *lcname; @@ -2483,9 +2483,11 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) / } name_len = strlen(module->name); - lcname = zend_string_alloc(name_len, module->type == MODULE_PERSISTENT); + lcname = zend_string_alloc(name_len, module_type == MODULE_PERSISTENT); zend_str_tolower_copy(ZSTR_VAL(lcname), module->name, name_len); + int module_number = zend_next_free_module(); + lcname = zend_new_interned_string(lcname); if ((module_ptr = zend_hash_add_ptr(&module_registry, lcname, module)) == NULL) { zend_error(E_CORE_WARNING, "Module \"%s\" is already loaded", module->name); @@ -2495,7 +2497,10 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) / module = module_ptr; EG(current_module) = module; - if (module->functions && zend_register_functions(NULL, module->functions, NULL, module->type)==FAILURE) { + module->module_number = module_number; + module->type = module_type; + + if (module->functions && zend_register_functions(NULL, module->functions, NULL, module_type)==FAILURE) { zend_hash_del(&module_registry, lcname); zend_string_release(lcname); EG(current_module) = NULL; @@ -2511,9 +2516,7 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) / ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry *module) /* {{{ */ { - module->module_number = zend_next_free_module(); - module->type = MODULE_PERSISTENT; - return zend_register_module_ex(module); + return zend_register_module_ex(module, MODULE_PERSISTENT); } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index acdc5b06b3d52..0bcce3ceb6e4f 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -375,7 +375,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend ZEND_API void zend_unregister_functions(const zend_function_entry *functions, int count, HashTable *function_table); ZEND_API zend_result zend_startup_module(zend_module_entry *module_entry); ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry *module_entry); -ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module); +ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module, int module_type); ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module); ZEND_API void zend_startup_modules(void); ZEND_API void zend_collect_module_handlers(void); diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c index 35e69b4a25dc3..ec1134cd7f2b3 100644 --- a/Zend/zend_builtin_functions.c +++ b/Zend/zend_builtin_functions.c @@ -59,9 +59,7 @@ zend_module_entry zend_builtin_module = { /* {{{ */ zend_result zend_startup_builtin_functions(void) /* {{{ */ { - zend_builtin_module.module_number = 0; - zend_builtin_module.type = MODULE_PERSISTENT; - return (EG(current_module) = zend_register_module_ex(&zend_builtin_module)) == NULL ? FAILURE : SUCCESS; + return (EG(current_module) = zend_register_module_ex(&zend_builtin_module, MODULE_PERSISTENT)) == NULL ? FAILURE : SUCCESS; } /* }}} */ diff --git a/ext/standard/dl.c b/ext/standard/dl.c index ebf8a3a507ebb..586862ec92ced 100644 --- a/ext/standard/dl.c +++ b/ext/standard/dl.c @@ -230,15 +230,14 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now) DL_UNLOAD(handle); return FAILURE; } - module_entry->type = type; - module_entry->module_number = zend_next_free_module(); - module_entry->handle = handle; - if ((module_entry = zend_register_module_ex(module_entry)) == NULL) { + if ((module_entry = zend_register_module_ex(module_entry, type)) == NULL) { DL_UNLOAD(handle); return FAILURE; } + module_entry->handle = handle; + if ((type == MODULE_TEMPORARY || start_now) && zend_startup_module_ex(module_entry) == FAILURE) { DL_UNLOAD(handle); return FAILURE; diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index 39befbd64ec32..9551e6f8ee92b 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -1314,16 +1314,14 @@ PHPDBG_API const char *phpdbg_load_module_or_extension(char **path, const char * goto quit; } - module_entry->type = MODULE_PERSISTENT; - module_entry->module_number = zend_next_free_module(); - module_entry->handle = handle; - - if ((module_entry = zend_register_module_ex(module_entry)) == NULL) { + if ((module_entry = zend_register_module_ex(module_entry, MODULE_PERSISTENT)) == NULL) { phpdbg_error("Unable to register module %s", *name); goto quit; } + module_entry->handle = handle; + if (zend_startup_module_ex(module_entry) == FAILURE) { phpdbg_error("Unable to startup module %s", module_entry->name);