From fd289bc947ce1fde98412e0da1db0e5db34bb954 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 20 May 2026 21:31:52 -0400 Subject: [PATCH] Fix GH-13230: phpdbg use-after-free at shutdown phpdbg_watch_element back-pointers to phpdbg_watchpoint_t went stale when the watchpoint was freed, defeating the GH-13681 NULL guards. phpdbg_destroy_watchpoints also iterated its hashes in MSHUTDOWN, after zend_mm_shutdown freed their emalloc backings: non-ASAN tolerated the read, ZTS ASAN aborted. NULL the back-pointer in phpdbg_clean_watch_element, tolerate NULL in phpdbg_backup_watch_element, unregister the freed element from watch_recreation in phpdbg_free_watch_element, and move the recreation drain and the btree plus hash reset into RSHUTDOWN so the work runs while emalloc memory is alive. Drop the late notices from the existing watch_*, gh15210_*, and bug73927 expected outputs since they were artifacts of reading freed memory. Fixes GH-13230 --- NEWS | 3 +++ sapi/phpdbg/phpdbg.c | 2 ++ sapi/phpdbg/phpdbg_watch.c | 25 +++++++++++++++++++++---- sapi/phpdbg/phpdbg_watch.h | 1 + sapi/phpdbg/tests/bug73927.phpt | 9 +-------- sapi/phpdbg/tests/gh15210_001.phpt | 2 +- sapi/phpdbg/tests/gh15210_002.phpt | 1 - sapi/phpdbg/tests/watch_001.phpt | 5 ++--- sapi/phpdbg/tests/watch_002.phpt | 5 ++--- sapi/phpdbg/tests/watch_003.phpt | 5 ++--- sapi/phpdbg/tests/watch_004.phpt | 5 ++--- sapi/phpdbg/tests/watch_005.phpt | 5 ++--- 12 files changed, 39 insertions(+), 29 deletions(-) diff --git a/NEWS b/NEWS index e37effff3afe..f76a2641d5cb 100644 --- a/NEWS +++ b/NEWS @@ -138,6 +138,9 @@ PHP NEWS . Mark Phar::buildFromIterator() base directory argument as a path. (ndossche) +- phpdbg: + . Fixed bug GH-13230 (use-after-free at phpdbg shutdown). (iliaal) + - Posix: . Added validity check to the flags argument for posix_access(). (arshidkv12) diff --git a/sapi/phpdbg/phpdbg.c b/sapi/phpdbg/phpdbg.c index 3c0d5e836dd0..75a27f72ee14 100644 --- a/sapi/phpdbg/phpdbg.c +++ b/sapi/phpdbg/phpdbg.c @@ -231,6 +231,8 @@ static PHP_RINIT_FUNCTION(phpdbg) /* {{{ */ static PHP_RSHUTDOWN_FUNCTION(phpdbg) /* {{{ */ { + phpdbg_destroy_watch_request_state(); + if (PHPDBG_G(stdin_file)) { fclose(PHPDBG_G(stdin_file)); PHPDBG_G(stdin_file) = NULL; diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index b6d5e543a19c..e0cbc192ade6 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -801,6 +801,7 @@ void phpdbg_dequeue_elements_for_recreation(void) { void phpdbg_clean_watch_element(phpdbg_watch_element *element); void phpdbg_free_watch_element(phpdbg_watch_element *element) { + zend_hash_del(&PHPDBG_G(watch_recreation), element->str); zend_string_release(element->str); if (element->name_in_parent) { zend_string_release(element->name_in_parent); @@ -856,6 +857,9 @@ void phpdbg_remove_watch_element(phpdbg_watch_element *element) { } void phpdbg_backup_watch_element(phpdbg_watch_element *element) { + if (!element->watch) { + return; + } memcpy(&element->backup, &element->watch->backup, /* element->watch->size */ sizeof(element->backup)); } @@ -974,6 +978,7 @@ void phpdbg_clean_watch_element(phpdbg_watch_element *element) { if (zend_hash_num_elements(elements) == 0) { phpdbg_remove_watchpoint(element->watch); } + element->watch = NULL; } } @@ -1501,15 +1506,27 @@ void phpdbg_setup_watchpoints(void) { #endif } -void phpdbg_destroy_watchpoints(void) { +void phpdbg_destroy_watch_request_state(void) { phpdbg_watch_element *element; - /* unconditionally free all remaining elements to avoid memory leaks */ ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) { phpdbg_automatic_dequeue_free(element); } ZEND_HASH_FOREACH_END(); - /* upon fatal errors etc. (i.e. CG(unclean_shutdown) == 1), some watchpoints may still be active. Ensure memory is not watched anymore for next run. Do not care about memory freeing here, shutdown is unclean and near anyway. */ + phpdbg_btree_init(&PHPDBG_G(watchpoint_tree), sizeof(void *) * 8); + phpdbg_btree_init(&PHPDBG_G(watch_HashTables), sizeof(void *) * 8); + + zend_hash_destroy(&PHPDBG_G(watch_elements)); + zend_hash_init(&PHPDBG_G(watch_elements), 8, NULL, NULL, 0); + zend_hash_destroy(&PHPDBG_G(watch_recreation)); + zend_hash_init(&PHPDBG_G(watch_recreation), 8, NULL, NULL, 0); + zend_hash_destroy(&PHPDBG_G(watch_free)); + zend_hash_init(&PHPDBG_G(watch_free), 8, NULL, NULL, 0); + zend_hash_destroy(&PHPDBG_G(watch_collisions)); + zend_hash_init(&PHPDBG_G(watch_collisions), 8, NULL, NULL, 0); +} + +void phpdbg_destroy_watchpoints(void) { phpdbg_purge_watchpoint_tree(); #ifdef HAVE_USERFAULTFD_WRITEFAULT @@ -1519,7 +1536,7 @@ void phpdbg_destroy_watchpoints(void) { } #endif - zend_hash_destroy(&PHPDBG_G(watch_elements)); PHPDBG_G(watch_elements).nNumOfElements = 0; /* phpdbg_watch_efree() is checking against this arrays size */ + zend_hash_destroy(&PHPDBG_G(watch_elements)); zend_hash_destroy(&PHPDBG_G(watch_recreation)); zend_hash_destroy(&PHPDBG_G(watch_free)); zend_hash_destroy(&PHPDBG_G(watch_collisions)); diff --git a/sapi/phpdbg/phpdbg_watch.h b/sapi/phpdbg/phpdbg_watch.h index 380140fd9f1f..99a896deba95 100644 --- a/sapi/phpdbg/phpdbg_watch.h +++ b/sapi/phpdbg/phpdbg_watch.h @@ -110,6 +110,7 @@ typedef struct { void phpdbg_setup_watchpoints(void); void phpdbg_destroy_watchpoints(void); +void phpdbg_destroy_watch_request_state(void); void phpdbg_purge_watchpoint_tree(void); #ifndef _WIN32 diff --git a/sapi/phpdbg/tests/bug73927.phpt b/sapi/phpdbg/tests/bug73927.phpt index bb54f4e17617..5c2231e5b173 100644 --- a/sapi/phpdbg/tests/bug73927.phpt +++ b/sapi/phpdbg/tests/bug73927.phpt @@ -1,11 +1,5 @@ --TEST-- Bug #73927 (phpdbg fails with windows error prompt at "watch array") ---SKIPIF-- - --PHPDBG-- b 19 r @@ -26,8 +20,7 @@ prompt> [Breakpoint #0 at %s:%d, hits: 2] 00021: } else { prompt> [Added watchpoint #0 for $value] prompt> [Added watchpoint #1 for $lower[0]] -prompt> [$lower[0] has been removed, removing watchpoint] -[$value has been removed, removing watchpoint] +prompt> --FILE-- 00002: header_register_callback(function() { echo "sent";}); 00003: $a = [0]; 00004: $a[0] = 1; -prompt> [$a[0] has been removed, removing watchpoint] +prompt> diff --git a/sapi/phpdbg/tests/gh15210_002.phpt b/sapi/phpdbg/tests/gh15210_002.phpt index 7848500a9e94..88cfeb135378 100644 --- a/sapi/phpdbg/tests/gh15210_002.phpt +++ b/sapi/phpdbg/tests/gh15210_002.phpt @@ -37,6 +37,5 @@ New value: 1 prompt> sent0 New value: 1 -[$a[0] has been removed, removing watchpoint] [Script ended normally] prompt> diff --git a/sapi/phpdbg/tests/watch_001.phpt b/sapi/phpdbg/tests/watch_001.phpt index 0a3d6e6c0e31..870cc85b4b83 100644 --- a/sapi/phpdbg/tests/watch_001.phpt +++ b/sapi/phpdbg/tests/watch_001.phpt @@ -40,9 +40,8 @@ prompt> [Breaking on watchpoint $b] Old value: New value: 2 >00008: -prompt> [$b has been removed, removing watchpoint recursively] -[Script ended normally] -prompt> +prompt> [Script ended normally] +prompt> --FILE-- [Added watchpoint #0 for $a[]] prompt> [Breaking on watchpoint $a[]] 1 elements were added to the array >00009: -prompt> [$a[] has been removed, removing watchpoint] -[Script ended normally] -prompt> +prompt> [Script ended normally] +prompt> --FILE-- [Breaking on watchpoint $a[0]] Old value: 2 New value: 3 >00009: -prompt> [$a[0] has been removed, removing watchpoint] -[Script ended normally] -prompt> +prompt> [Script ended normally] +prompt> --FILE-- [Breaking on watchpoint $a] Old value: aa New value: ab >00006: -prompt> [$a has been removed, removing watchpoint] -[Script ended normally] -prompt> +prompt> [Script ended normally] +prompt> --FILE-- 00008: exit; 00009: -prompt> [$a has been removed, removing watchpoint recursively] -[Script ended normally] -prompt> +prompt> [Script ended normally] +prompt> --FILE--