Skip to content

Fix GH-23106: mb_strpos() reads past the end of a haystack ending in a truncated UTF-8 sequence - #25123

Closed
VishnuSrivatsava wants to merge 1 commit into
php:PHP-8.2from
VishnuSrivatsava:fix/gh23106-mb-strpos-8.2
Closed

VishnuSrivatsava wants to merge 1 commit into
php:PHP-8.2from
VishnuSrivatsava:fix/gh23106-mb-strpos-8.2

Conversation

@VishnuSrivatsava

Copy link
Copy Markdown

Fixes the PHP-8.2 part of #25025 and GH-23106.

Backport of master commit cc22056 (PR #23107, credited to Lazizbek Ergashev). As analyzed in #25025, on this branch the offset walk lives in mbfl_find_offset_utf8() in libmbfl, so the master patch had to be rewritten for the different function and const unsigned char * pointers rather than applied directly.

The forward walk can land past the end of the haystack on a truncated multi-byte sequence, and the returned pointer is then used by zend_memnstr() despite violating its end >= p invariant. The clamp restores the invariant for the forward path and for the needle-offset walk feeding zend_memnrstr() on the reverse path.

Changes:

Verification on this branch (macOS arm64, --enable-debug + AddressSanitizer):

  • Pre-fix: mb_strpos("AA\xf0\x90", "xyz", 3) aborts with Assertion failed: (end >= p), function zend_memnstr, file zend_operators.h, line 179.
  • Post-fix: assertion gone, regression test passes, full ext/mbstring suite passes (327 passed, 0 failed).

Not touching NEWS, following the branch convention where release managers add entries at packaging time. 8.2 is in security-only support until 2026-12-31; since this is an out-of-bounds read, it should qualify for a security release, but that call belongs to release managers. The PHP-8.3 backport is in #25121.

…in a truncated UTF-8 sequence

Backport of master commit cc22056 (PR php#23107), requested in phpGH-25025.
On this branch the offset walk lives in mbfl_find_offset_utf8() in
libmbfl. The forward walk can land past the end of the haystack on a
truncated multi-byte sequence, and the returned pointer is then used by
zend_memnstr() despite violating its end >= pointer invariant. Clamp
the pointer to the end of the haystack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant