Repository navigation
Conversation
Backport of 509e27b (Closes phpGH-23499; fix by Ilia Alshanetsky). When a \k<name> backref in the replacement lacks its closing delimiter, two error paths in mb_regex_substitute() advance p past eos and the following smart_str_appendl() copies the overshoot (a NUL terminator, or heap bytes past the string) into the result. Clamp both sites to eos, identical to PHP-8.4+. The fix shipped in 8.4.26 but was never backported to the still-supported 8.2 and 8.3 branches.
Contributor
Author
|
No, don't know them at all. We probably read alike because both PRs leaned on an LLM for an mbstring OOB backport — and "a fix that landed in 8.4 but never got backported" is an obvious thing to go looking for, so same idea the same day isn't that surprising. I did build and test this one for real though: 8.2 and 8.3 from source, the upstream test fails before the clamps and passes after, full |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports the
mb_ereg_replace()\k<name>out-of-bounds read fix (509e27b, Closes GH-23499; fix by Ilia Alshanetsky) to the still-supported PHP-8.2 and PHP-8.3 branches, as requested in GH-25024.When a
\k<name>backref in the replacement string has no closing delimiter,mb_regex_substitute()advancesppasteosat two sites and the followingsmart_str_appendl(pbuf, sp, p - sp)copies the overshoot (a NUL terminator, or heap bytes past the string) into the returned result. Both sites are clamped toeos, identical to PHP-8.4+.Opened against PHP-8.2 so it merges up into 8.3 (8.4 already has the fix). If 8.2 is considered too close to EOL to warrant it, happy to retarget to PHP-8.3.
Testing (built from source, NTS,
--enable-mbstring):ext/mbstring/tests/mb_ereg_replace_kname_unterminated_nul.phpt(the upstream test): FAILS on clean 8.2 and 8.3 (output carries a trailing NUL / overshoot), PASSES after the clamps.ext/mbstring/tests: 8.2 = 406 passed / 0 failed (2 skipped), 8.3 = 423 passed / 0 failed (2 skipped).Drafted with an LLM; I reviewed and tested it myself.