Skip to content

Commit 8a812c3

Browse files
committedSep 20, 2023
Fix GH-12215: Module entry being overwritten causes type errors in ext/dom (PHP 8.4)
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. Closes GH-12246.
1 parent aab6bbc commit 8a812c3

File tree

6 files changed

+22
-19
lines changed

6 files changed

+22
-19
lines changed
 

‎UPGRADING.INTERNALS

+5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES
1414
1. Internal API changes
1515
========================
1616

17+
* zend_register_module_ex() now takes an additional int module_type argument.
18+
This function will also assign the module number and type, there is no need
19+
to do this at the call site anymore. Writing the handle should happen after
20+
successful registration.
21+
1722
========================
1823
2. Build system changes
1924
========================

‎Zend/zend_API.c

+9-6
Original file line numberDiff line numberDiff line change
@@ -2446,7 +2446,7 @@ ZEND_API void zend_destroy_modules(void) /* {{{ */
24462446
}
24472447
/* }}} */
24482448

2449-
ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) /* {{{ */
2449+
ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module, int module_type) /* {{{ */
24502450
{
24512451
size_t name_len;
24522452
zend_string *lcname;
@@ -2483,9 +2483,11 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) /
24832483
}
24842484

24852485
name_len = strlen(module->name);
2486-
lcname = zend_string_alloc(name_len, module->type == MODULE_PERSISTENT);
2486+
lcname = zend_string_alloc(name_len, module_type == MODULE_PERSISTENT);
24872487
zend_str_tolower_copy(ZSTR_VAL(lcname), module->name, name_len);
24882488

2489+
int module_number = zend_next_free_module();
2490+
24892491
lcname = zend_new_interned_string(lcname);
24902492
if ((module_ptr = zend_hash_add_ptr(&module_registry, lcname, module)) == NULL) {
24912493
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) /
24952497
module = module_ptr;
24962498
EG(current_module) = module;
24972499

2498-
if (module->functions && zend_register_functions(NULL, module->functions, NULL, module->type)==FAILURE) {
2500+
module->module_number = module_number;
2501+
module->type = module_type;
2502+
2503+
if (module->functions && zend_register_functions(NULL, module->functions, NULL, module_type)==FAILURE) {
24992504
zend_hash_del(&module_registry, lcname);
25002505
zend_string_release(lcname);
25012506
EG(current_module) = NULL;
@@ -2511,9 +2516,7 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module) /
25112516

25122517
ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry *module) /* {{{ */
25132518
{
2514-
module->module_number = zend_next_free_module();
2515-
module->type = MODULE_PERSISTENT;
2516-
return zend_register_module_ex(module);
2519+
return zend_register_module_ex(module, MODULE_PERSISTENT);
25172520
}
25182521
/* }}} */
25192522

‎Zend/zend_API.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
375375
ZEND_API void zend_unregister_functions(const zend_function_entry *functions, int count, HashTable *function_table);
376376
ZEND_API zend_result zend_startup_module(zend_module_entry *module_entry);
377377
ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry *module_entry);
378-
ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module);
378+
ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module, int module_type);
379379
ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module);
380380
ZEND_API void zend_startup_modules(void);
381381
ZEND_API void zend_collect_module_handlers(void);

‎Zend/zend_builtin_functions.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ zend_module_entry zend_builtin_module = { /* {{{ */
5959

6060
zend_result zend_startup_builtin_functions(void) /* {{{ */
6161
{
62-
zend_builtin_module.module_number = 0;
63-
zend_builtin_module.type = MODULE_PERSISTENT;
64-
return (EG(current_module) = zend_register_module_ex(&zend_builtin_module)) == NULL ? FAILURE : SUCCESS;
62+
return (EG(current_module) = zend_register_module_ex(&zend_builtin_module, MODULE_PERSISTENT)) == NULL ? FAILURE : SUCCESS;
6563
}
6664
/* }}} */
6765

‎ext/standard/dl.c

+3-4
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,14 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now)
230230
DL_UNLOAD(handle);
231231
return FAILURE;
232232
}
233-
module_entry->type = type;
234-
module_entry->module_number = zend_next_free_module();
235-
module_entry->handle = handle;
236233

237-
if ((module_entry = zend_register_module_ex(module_entry)) == NULL) {
234+
if ((module_entry = zend_register_module_ex(module_entry, type)) == NULL) {
238235
DL_UNLOAD(handle);
239236
return FAILURE;
240237
}
241238

239+
module_entry->handle = handle;
240+
242241
if ((type == MODULE_TEMPORARY || start_now) && zend_startup_module_ex(module_entry) == FAILURE) {
243242
DL_UNLOAD(handle);
244243
return FAILURE;

‎sapi/phpdbg/phpdbg_prompt.c

+3-5
Original file line numberDiff line numberDiff line change
@@ -1314,16 +1314,14 @@ PHPDBG_API const char *phpdbg_load_module_or_extension(char **path, const char *
13141314
goto quit;
13151315
}
13161316

1317-
module_entry->type = MODULE_PERSISTENT;
1318-
module_entry->module_number = zend_next_free_module();
1319-
module_entry->handle = handle;
1320-
1321-
if ((module_entry = zend_register_module_ex(module_entry)) == NULL) {
1317+
if ((module_entry = zend_register_module_ex(module_entry, MODULE_PERSISTENT)) == NULL) {
13221318
phpdbg_error("Unable to register module %s", *name);
13231319

13241320
goto quit;
13251321
}
13261322

1323+
module_entry->handle = handle;
1324+
13271325
if (zend_startup_module_ex(module_entry) == FAILURE) {
13281326
phpdbg_error("Unable to startup module %s", module_entry->name);
13291327

0 commit comments

Comments
 (0)