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

Author: iliaal

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)
 

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;

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));

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

sapi/phpdbg/tests/bug73927.phpt

@@ -1,11 +1,5 @@
 --TEST--
 Bug #73927 (phpdbg fails with windows error prompt at "watch array")
---SKIPIF--
-<?php
-if (getenv('SKIP_ASAN')) {
-    die("skip intentionally causes segfaults");
-}
-?>
 --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--
 <?php
 

sapi/phpdbg/tests/gh15210_001.phpt

@@ -33,4 +33,4 @@ New value: 1
 >00002: header_register_callback(function() { echo "sent";});
  00003: $a = [0];
  00004: $a[0] = 1;
-prompt> [$a[0] has been removed, removing watchpoint]
+prompt>

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>

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--
 <?php
 

sapi/phpdbg/tests/watch_002.phpt

@@ -24,9 +24,8 @@ prompt> [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--
 <?php
 

sapi/phpdbg/tests/watch_003.phpt

@@ -31,9 +31,8 @@ prompt> [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--
 <?php