[php-src] Issue #19543: Memory unsafety with `HashTable`s treated as weak map

Issue: https://github.com/php/php-src/issues/19543 Author: TimWolla ### Description Apply the following patch to zend_test: ``` diff --git i/ext/zend_test/php_test.h w/ext/zend_test/php_test.h index 09391d8137a..7ec6f543123 100644 --- i/ext/zend_test/php_test.h +++ w/ext/zend_test/php_test.h @@ -50,7 +50,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test) int observer_fiber_switch; int observer_fiber_destroy; int observer_execute_internal; - HashTable global_weakmap; + HashTable *global_weakmap; int replace_zend_execute_ex; int register_passes; bool print_stderr_mshutdown; diff --git i/ext/zend_test/test.c w/ext/zend_test/test.c index af60dffb4bf..1ca94983465 100644 --- i/ext/zend_test/test.c +++ w/ext/zend_test/test.c @@ -381,7 +381,7 @@ static ZEND_FUNCTION(zend_weakmap_attach) Z_PARAM_ZVAL(value) ZEND_PARSE_PARAMETERS_END(); - if (zend_weakrefs_hash_add(&ZT_G(global_weakmap), obj, value)) { + if (zend_weakrefs_hash_add(ZT_G(global_weakmap), obj, value)) { Z_TRY_ADDREF_P(value); RETURN_TRUE; } @@ -396,13 +396,13 @@ static ZEND_FUNCTION(zend_weakmap_remove) Z_PARAM_OBJ(obj) ZEND_PARSE_PARAMETERS_END(); - RETURN_BOOL(zend_weakrefs_hash_del(&ZT_G(global_weakmap), obj) == SUCCESS); + RETURN_BOOL(zend_weakrefs_hash_del(ZT_G(global_weakmap), obj) == SUCCESS); } static ZEND_FUNCTION(zend_weakmap_dump) { ZEND_PARSE_PARAMETERS_NONE(); - RETURN_ARR(zend_array_dup(&ZT_G(global_weakmap))); + RETURN_ARR(zend_array_dup(ZT_G(global_weakmap))); } static ZEND_FUNCTION(zend_get_current_func_name) @@ -1424,7 +1424,8 @@ PHP_MSHUTDOWN_FUNCTION(zend_test) PHP_RINIT_FUNCTION(zend_test) { - zend_hash_init(&ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0); + ZT_G(global_weakmap) = emalloc(sizeof(HashTable)); + zend_hash_init(ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0); ZT_G(observer_nesting_depth) = 0; zend_test_mm_custom_handlers_rinit(); return SUCCESS; @@ -1432,7 +1433,8 @@ PHP_RINIT_FUNCTION(zend_test) PHP_RSHUTDOWN_FUNCTION(zend_test) { - zend_weakrefs_hash_destroy(&ZT_G(global_weakmap)); + zend_weakrefs_hash_destroy(ZT_G(global_weakmap)); + efree(ZT_G(global_weakmap)); if (ZT_G(zend_test_heap)) { free(ZT_G(zend_test_heap)); ``` The following code: ```php <?php $e = new Exception(); $a = new stdClass(); zend_weakmap_attach($e, $a); unset($a); gc_collect_cycles(); ``` Resulted in this output: ``` ==884545== Memcheck, a memory error detector ==884545== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==884545== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info ==884545== Command: sapi/cli/php weakmap.php ==884545== ==884545== Invalid read of size 4 ==884545== at 0xB77ACE: gc_scan_black (zend_gc.c:868) ==884545== by 0xB78DD9: gc_scan (zend_gc.c:1301) ==884545== by 0xB79419: gc_scan_roots (zend_gc.c:1448) ==884545== by 0xB7A90C: zend_gc_collect_cycles (zend_gc.c:2009) ==884545== by 0xA8C145: zif_gc_collect_cycles (zend_builtin_functions.c:170) ==884545== by 0xAE99CF: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1344) ==884545== by 0xB6B1CF: execute_ex (zend_vm_execute.h:58930) ==884545== by 0xB70C35: zend_execute (zend_vm_execute.h:64398) ==884545== by 0xC14DCB: zend_execute_script (zend.c:1976) ==884545== by 0x9B3931: php_execute_script_ex (main.c:2647) ==884545== by 0x9B3AB7: php_execute_script (main.c:2687) ==884545== by 0xC174E5: do_cli (php_cli.c:952) ==884545== Address 0x7bc488c is 4 bytes after a block of size 56 alloc'd ==884545== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==884545== by 0xA65AC0: __zend_malloc (zend_alloc.c:3561) ==884545== by 0xA63CAB: _emalloc (zend_alloc.c:2798) ==884545== by 0x9A925E: zm_activate_zend_test (test.c:1427) ==884545== by 0xA7392E: zend_activate_modules (zend_API.c:3396) ==884545== by 0x9B2853: php_request_startup (main.c:1946) ==884545== by 0xC1734A: do_cli (php_cli.c:920) ==884545== by 0xC18544: main (php_cli.c:1363) ==884545== ==884545== Invalid read of size 4 ==884545== at 0xB77B23: gc_scan_black (zend_gc.c:882) ==884545== by 0xB78DD9: gc_scan (zend_gc.c:1301) ==884545== by 0xB79419: gc_scan_roots (zend_gc.c:1448) ==884545== by 0xB7A90C: zend_gc_collect_cycles (zend_gc.c:2009) ==884545== by 0xA8C145: zif_gc_collect_cycles (zend_builtin_functions.c:170) ==884545== by 0xAE99CF: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1344) ==884545== by 0xB6B1CF: execute_ex (zend_vm_execute.h:58930) ==884545== by 0xB70C35: zend_execute (zend_vm_execute.h:64398) ==884545== by 0xC14DCB: zend_execute_script (zend.c:1976) ==884545== by 0x9B3931: php_execute_script_ex (main.c:2647) ==884545== by 0x9B3AB7: php_execute_script (main.c:2687) ==884545== by 0xC174E5: do_cli (php_cli.c:952) ==884545== Address 0x7bc488c is 4 bytes after a block of size 56 alloc'd ==884545== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==884545== by 0xA65AC0: __zend_malloc (zend_alloc.c:3561) ==884545== by 0xA63CAB: _emalloc (zend_alloc.c:2798) ==884545== by 0x9A925E: zm_activate_zend_test (test.c:1427) ==884545== by 0xA7392E: zend_activate_modules (zend_API.c:3396) ==884545== by 0x9B2853: php_request_startup (main.c:1946) ==884545== by 0xC1734A: do_cli (php_cli.c:920) ==884545== by 0xC18544: main (php_cli.c:1363) ==884545== ==884545== ==884545== HEAP SUMMARY: ==884545== in use at exit: 0 bytes in 0 blocks ==884545== total heap usage: 18,859 allocs, 18,859 frees, 3,708,637 bytes allocated ==884545== ==884545== All heap blocks were freed -- no leaks are possible ==884545== ==884545== For lists of detected and suppressed errors, rerun with: -s ==884545== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ``` But I expected this output instead: No memory unsafety. -------------- The internal weak map API for bare HashTables was added by @bwoebi in 471102edcd19c79be2541188ae8a211a2cbc85bb. I've shortly checked with him and he indicated that this got broken in cbf67e4feee6b4a0eb23d3530efa58c682d081c5 by @arnaud-lb, because that commit treats anything passed to zend_weakrefs_hash_add() as an object of zend_ce_weakref (i.e. a zend_weakmap struct rather than a bare HashTable). ### PHP Version ```plain git master ``` ### Operating System _No response_