From 20259f847f767575a99816018c20374dbb734bb9 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 15 Jul 2026 16:01:28 -0400 Subject: [PATCH] Fix phpdbg over-read when watching a packed array element phpdbg watched every array element as a Bucket, but packed arrays store bare zvals and de-indirected stack variables aren't buckets, so reading Bucket.h/.key over-read the neighbour and a sibling write tripped a phantom break. Watch such elements as WATCH_ON_ZVAL and compare the type info first, reading the value union only for initialised slots so an uninitialised CV is never compared by value. A separate pre-existing bug crashed 32-bit: phpdbg_btree_insert_or_update linked each new node into the tree before initialising its child pointers, so a write to a node on a watched page faulted into the watchpoint handler, which walked the half-built node. Build the node fully, then publish it with a single store. Closes GH-22756 --- sapi/phpdbg/phpdbg_btree.c | 13 +++++--- sapi/phpdbg/phpdbg_watch.c | 54 +++++++++++++++++++++++++------- sapi/phpdbg/phpdbg_watch.h | 1 + sapi/phpdbg/tests/watch_005.phpt | 9 ------ sapi/phpdbg/tests/watch_006.phpt | 6 ---- 5 files changed, 52 insertions(+), 31 deletions(-) diff --git a/sapi/phpdbg/phpdbg_btree.c b/sapi/phpdbg/phpdbg_btree.c index f7c554884d9d..788e9b464c14 100644 --- a/sapi/phpdbg/phpdbg_btree.c +++ b/sapi/phpdbg/phpdbg_btree.c @@ -156,13 +156,18 @@ int phpdbg_btree_insert_or_update(phpdbg_btree *tree, zend_ulong idx, void *ptr, } { - phpdbg_btree_branch *memory = *branch = pemalloc((i + 2) * sizeof(phpdbg_btree_branch), tree->persistent); + phpdbg_btree_branch *memory = pemalloc((i + 2) * sizeof(phpdbg_btree_branch), tree->persistent); + phpdbg_btree_branch *node = memory; do { - (*branch)->branches[!((idx >> i) % 2)] = NULL; - branch = &(*branch)->branches[(idx >> i) % 2]; - *branch = ++memory; + node->branches[!((idx >> i) % 2)] = NULL; + node->branches[(idx >> i) % 2] = node + 1; + node++; } while (i--); + node->result.idx = idx; + node->result.ptr = ptr; + *branch = memory; tree->count++; + return SUCCESS; } } else if (!(flags & PHPDBG_BTREE_UPDATE)) { return FAILURE; diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index ed6e6dfc70b9..1e67d2c57670 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -133,6 +133,16 @@ const phpdbg_command_t phpdbg_watch_commands[] = { #define HT_WATCH_HT(watch) HT_PTR_HT((watch)->addr.ptr) /* ### PRINTING POINTER DIFFERENCES ### */ +static bool phpdbg_check_zval_watch_diff(zval *oldPtr, zval *newPtr) { + if (Z_TYPE_INFO_P(oldPtr) != Z_TYPE_INFO_P(newPtr)) { + return true; + } + if (Z_TYPE_P(oldPtr) < IS_LONG) { + return false; + } + return memcmp(oldPtr, newPtr, sizeof(zend_value)) != 0; +} + bool phpdbg_check_watch_diff(phpdbg_watchtype type, void *oldPtr, void *newPtr) { switch (type) { case WATCH_ON_BUCKET: @@ -142,7 +152,7 @@ bool phpdbg_check_watch_diff(phpdbg_watchtype type, void *oldPtr, void *newPtr) /* Fall through to also compare the value from the bucket. */ ZEND_FALLTHROUGH; case WATCH_ON_ZVAL: - return memcmp(oldPtr, newPtr, sizeof(zend_value) + sizeof(uint32_t) /* value + typeinfo */) != 0; + return phpdbg_check_zval_watch_diff((zval *) oldPtr, (zval *) newPtr); case WATCH_ON_HASHTABLE: return zend_hash_num_elements(HT_PTR_HT(oldPtr)) != zend_hash_num_elements(HT_PTR_HT(newPtr)); case WATCH_ON_REFCOUNTED: @@ -568,9 +578,26 @@ phpdbg_watch_element *phpdbg_add_watch_element(phpdbg_watchpoint_t *watch, phpdb return element; } -phpdbg_watch_element *phpdbg_add_bucket_watch_element(Bucket *bucket, phpdbg_watch_element *element, bool *is_new) { +static bool phpdbg_zval_is_bucket_of(HashTable *ht, zval *zv) { + if (!ht || HT_IS_PACKED(ht)) { + return false; + } + if ((uintptr_t) zv < (uintptr_t) ht->arData + || (uintptr_t) zv >= (uintptr_t) (ht->arData + ht->nNumUsed)) { + return false; + } + ZEND_ASSERT(((uintptr_t) zv - (uintptr_t) ht->arData) % sizeof(Bucket) == 0); + return true; +} + +phpdbg_watch_element *phpdbg_add_bucket_watch_element(zval *zv, phpdbg_watch_element *element, bool *is_new) { phpdbg_watchpoint_t watch; - phpdbg_set_bucket_watchpoint(bucket, &watch); + + if (phpdbg_zval_is_bucket_of(element->parent_container, zv)) { + phpdbg_set_bucket_watchpoint((Bucket *) zv, &watch); + } else { + phpdbg_set_zval_watchpoint(zv, &watch); + } bool added_new; phpdbg_watch_element *added = phpdbg_add_watch_element(&watch, element, &added_new); if (added_new) { @@ -637,7 +664,7 @@ void phpdbg_add_recursive_watch_from_ht(phpdbg_watch_element *element, zend_long return; } bool added_new; - phpdbg_add_bucket_watch_element((Bucket *) zv, child, &added_new); + phpdbg_add_bucket_watch_element(zv, child, &added_new); if (added_new) { zend_hash_add_ptr(&element->child_container, child->str, child); } @@ -697,7 +724,7 @@ void phpdbg_recurse_watch_element(phpdbg_watch_element *element) { } void phpdbg_watch_parent_ht(phpdbg_watch_element *element) { - if (element->watch->type == WATCH_ON_BUCKET) { + if (element->watch->type == WATCH_ON_BUCKET || element->watch->type == WATCH_ON_ZVAL) { phpdbg_btree_result *res; phpdbg_watch_ht_info *hti; ZEND_ASSERT(element->parent_container); @@ -716,14 +743,17 @@ void phpdbg_watch_parent_ht(phpdbg_watch_element *element) { hti = (phpdbg_watch_ht_info *) res->ptr; } - zend_hash_add_ptr(&hti->watches, element->name_in_parent, element); + if (zend_hash_add_ptr(&hti->watches, element->name_in_parent, element)) { + element->flags |= PHPDBG_WATCH_HT_REGISTERED; + } } } void phpdbg_unwatch_parent_ht(phpdbg_watch_element *element) { - if (element->watch && element->watch->type == WATCH_ON_BUCKET) { + if (element->flags & PHPDBG_WATCH_HT_REGISTERED) { phpdbg_btree_result *res = phpdbg_btree_find(&PHPDBG_G(watch_HashTables), (zend_ulong) element->parent_container); ZEND_ASSERT(element->parent_container); + element->flags &= ~PHPDBG_WATCH_HT_REGISTERED; if (res) { phpdbg_watch_ht_info *hti = res->ptr; @@ -809,7 +839,7 @@ bool phpdbg_try_re_adding_watch_element(zval *parent, phpdbg_watch_element *elem return false; } element->parent_container = ht; - element = phpdbg_add_bucket_watch_element((Bucket *) zv, element, NULL); + element = phpdbg_add_bucket_watch_element(zv, element, NULL); } else { return false; } @@ -1320,7 +1350,7 @@ static phpdbg_watch_add_result phpdbg_create_simple_watchpoint(zval *zv, phpdbg_ phpdbg_watch_element *added; (*element)->flags = PHPDBG_WATCH_SIMPLE; bool added_new; - added = phpdbg_add_bucket_watch_element((Bucket *) zv, *element, &added_new); + added = phpdbg_add_bucket_watch_element(zv, *element, &added_new); if (!added_new) { *element = added; return PHPDBG_WATCH_ADD_DUPLICATE; @@ -1344,7 +1374,7 @@ static phpdbg_watch_add_result phpdbg_create_array_watchpoint(zval *zv, phpdbg_w element->str = str; element->flags = PHPDBG_WATCH_IMPLICIT; bool added_new; - added = phpdbg_add_bucket_watch_element((Bucket *) orig_zv, element, &added_new); + added = phpdbg_add_bucket_watch_element(orig_zv, element, &added_new); if (!added_new) { *element_ptr = added; return PHPDBG_WATCH_ADD_DUPLICATE; @@ -1372,7 +1402,7 @@ static phpdbg_watch_add_result phpdbg_create_recursive_watchpoint(zval *zv, phpd (*element)->flags = PHPDBG_WATCH_RECURSIVE | PHPDBG_WATCH_RECURSIVE_ROOT; (*element)->child = NULL; bool added_new; - added = phpdbg_add_bucket_watch_element((Bucket *) zv, *element, &added_new); + added = phpdbg_add_bucket_watch_element(zv, *element, &added_new); if (!added_new) { *element = added; return PHPDBG_WATCH_ADD_DUPLICATE; @@ -1451,7 +1481,7 @@ static int phpdbg_watchpoint_parse_step(char *name, size_t namelen, char *key, s element->name_in_parent = zend_string_init(key, keylen, 0); element->parent_container = parent; element->parent = PHPDBG_G(watch_tmp); - element = phpdbg_add_bucket_watch_element((Bucket *) zv, element, NULL); + element = phpdbg_add_bucket_watch_element(zv, element, NULL); efree(name); efree(key); diff --git a/sapi/phpdbg/phpdbg_watch.h b/sapi/phpdbg/phpdbg_watch.h index e87dd77f0543..9f2549a96a52 100644 --- a/sapi/phpdbg/phpdbg_watch.h +++ b/sapi/phpdbg/phpdbg_watch.h @@ -54,6 +54,7 @@ typedef enum { #define PHPDBG_WATCH_NORMAL (PHPDBG_WATCH_SIMPLE | PHPDBG_WATCH_RECURSIVE) #define PHPDBG_WATCH_IMPLICIT 0x10 #define PHPDBG_WATCH_RECURSIVE_ROOT 0x20 +#define PHPDBG_WATCH_HT_REGISTERED 0x40 typedef struct _phpdbg_watch_collision phpdbg_watch_collision; diff --git a/sapi/phpdbg/tests/watch_005.phpt b/sapi/phpdbg/tests/watch_005.phpt index aacc158f6174..d6bae9d3f3fa 100644 --- a/sapi/phpdbg/tests/watch_005.phpt +++ b/sapi/phpdbg/tests/watch_005.phpt @@ -1,14 +1,5 @@ --TEST-- Test proper watch comparisons when having multiple levels of indirection from a zval to its value ---SKIPIF-- - --PHPDBG-- b 3 r diff --git a/sapi/phpdbg/tests/watch_006.phpt b/sapi/phpdbg/tests/watch_006.phpt index bf38b8eff1fb..21b77a982d63 100644 --- a/sapi/phpdbg/tests/watch_006.phpt +++ b/sapi/phpdbg/tests/watch_006.phpt @@ -11,7 +11,6 @@ c - q --EXPECTF-- [Successful compilation of %s] @@ -48,11 +47,6 @@ prompt> [Element 1 has been added to watchpoint] 00010: prompt> [Breaking on watchpoint $b] Old value inaccessible or destroyed -New value (reference): Array ([0] => 2,[1] => 3) ->00009: $b = &$c; - 00010: -prompt> [Breaking on watchpoint $b] -Old value inaccessible or destroyed New value (reference): Array ([0] => 1) >00010: prompt> [$b has been removed, removing watchpoint recursively]