Fix GH-22779: mb_strrpos() wrong result in a non-UTF-8 encoding#22780
Fix GH-22779: mb_strrpos() wrong result in a non-UTF-8 encoding#22780eyupcanakman wants to merge 1 commit into
Conversation
mb_find_strpos() converts the haystack and needle to UTF-8 and does the length and offset math on the converted strings. The negative-offset branch computed the needle length from the raw needle instead of needle_u8. mb_fast_strlen_utf8() reads those bytes as UTF-8, so the count is wrong whenever they do not decode one to one. A byte in 0x80-0xBF (ISO-8859-1) undercounts, and a wider encoding such as an ASCII needle in UTF-16 overcounts, so the reverse-search window is shifted. Use needle_u8, the converted needle the rest of the branch already searches with.
|
Hmm! Interesting! I wrote that code in 2022. Now that you point out the mistake, it seems perfectly obvious, but it wasn't then. I fuzzed that code very hard, and the fuzzer caught a lot of mistakes, but not this one. Maybe my fuzzer's assertions weren't strict enough. |
|
This bug is present in PHP 8.3 but not 8.2. PHP 8.3 currently gets security fixes only. @eyupcanakman Can you think of any plausible reason why this should be considered a security issue? If not, then targeting PHP 8.4 (as you have done) is right. |
|
No, I don't think so. The wrong return value is a correctness bug with no memory-safety angle, so 8.4 is the right target. |
|
OK, well, all tests are passing (as expected). |
youkidearitai
left a comment
youkidearitai
left a comment
There was a problem hiding this comment.
Nothing, LGTM.
Thanks!
3 participants
mb_strrpos()with a negative offset returns the wrong position (orfalse) when the encoding is not UTF-8.mb_find_strpos()converts the haystack and needle to UTF-8 and does its length math on the converted strings, but the negative-offset branch measured the needle length from the rawneedle.mb_fast_strlen_utf8()counts those bytes as UTF-8, so the length is wrong when the source bytes are not their own UTF-8 form. A byte in 0x80-0xBF makes it too low and an ASCII needle in UTF-16 makes it too high, shifting the reverse-search window.The fix measures the length from
needle_u8. The test covers ISO-8859-1, Windows-1252, Shift_JIS and UTF-16.Fixes GH-22779