phpdbg: Fix off-by-one in phpdbg_safe_class_lookup() signal-safe class lookup#22593
Conversation
The title of the PR implies there is a bug. That sentence is much less clear; and the off-by-one isn't clearly described either. It this an actual bug that should be fixed in PHP 8.4? |
lc_length was set to name_length + 1, one past the actual lowercased string length, so zend_hash_str_find_ptr() (which expects a strlen-style length) never matched an existing class entry. This made class lookups during phpdbg's signal-handler interruption path always fail silently. Replaced the manual emalloc/tolower/efree dance with zend_hash_str_find_ptr_lc(), which performs the lowercase copy and lookup in one call and removes the surface for this class of bug.
jorgsowa
requested review from
Girgias,
SakiTakamachi,
TimWolla,
adoy,
arnaud-lb,
derickr,
devnexen,
dstogov and
kocsismate
as code owners
jorgsowa
force-pushed
the
fix-phpdbg-safe-class-lookup
branch
from
8756924 to
146fc94
Compare
|
I did the wrong order of cherry-picking. I'm sorry for the additional reviews. I cannot remove any reviewer now. @TimWolla, I modified the description to reflect the bug. I checked the branch 8.4, and it's also buggy, so I pointed to 8.4. |
TimWolla
removed request for
Girgias,
SakiTakamachi,
TimWolla,
adoy,
arnaud-lb,
derickr,
devnexen,
dstogov and
kocsismate
Thanks, this makes sense. Is this a user-facing bug - i.e. does this require a NEWS entry? Do you have a suggested NEWS entry text? |
|
Thanks. I updated the NEWS section. Unfortunately, it's not possible to create a phpt regression test case for this kind of bug. |
* PHP-8.4: phpdbg: Fix off-by-one in phpdbg_safe_class_lookup() signal-safe class lookup (#22593)
* PHP-8.5: phpdbg: Fix off-by-one in phpdbg_safe_class_lookup() signal-safe class lookup (#22593)
2 participants
lc_lengthwas set toname_length + 1(including the null terminator) in both branches, butzend_hash_str_find_ptr()requires an exact length match. Usingzend_hash_str_find_ptr_lc()with the correctname_lengthfixes it.