Skip to content

Fix phpdbg over-read when watching a packed array element #22756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iliaal merged 1 commit into from
Jul 24, 2026
Merged

Fix phpdbg over-read when watching a packed array element #22756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions sapi/phpdbg/phpdbg_btree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
54 changes: 42 additions & 12 deletions sapi/phpdbg/phpdbg_watch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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);
}
Comment thread
iluuu1994 marked this conversation as resolved.
bool added_new;
phpdbg_watch_element *added = phpdbg_add_watch_element(&watch, element, &added_new);
if (added_new) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions sapi/phpdbg/phpdbg_watch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
9 changes: 0 additions & 9 deletions sapi/phpdbg/tests/watch_005.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
--TEST--
Test proper watch comparisons when having multiple levels of indirection from a zval to its value
--SKIPIF--
<?php
if (PHP_INT_SIZE == 4) {
die("xfail There may be flaws in the implementation of watchpoints that cause failures");
}
if (getenv('SKIP_ASAN')) {
die("skip intentionally causes segfaults");
}
?>
--PHPDBG--
b 3
r
Expand Down
6 changes: 0 additions & 6 deletions sapi/phpdbg/tests/watch_006.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ c




q
--EXPECTF--
[Successful compilation of %s]
Expand Down Expand Up @@ -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]
Expand Down
Loading