Pr1116 symbol replace - #1119
Merged
Merged
Conversation
Previously, the matched string returned by exec() was assumed to be a substring of the subject at the reported index. njs_string_get_substitution() converted "index + matched length" to a byte offset without a bounds check, and the replace loop advanced the end of the last match by the byte size of the matched string instead of its character length. As a result the $' expansion read outside the subject, and on a multibyte subject the result could start in the middle of a UTF-8 sequence. The fix is to pass the head and tail slices of the subject to njs_string_get_substitution() and to track the end of the last match as a character index. The function no longer converts indexes to offsets, so the out of range conversion is prevented by construction. This is an extended fix for nginx#1116 pull request on Github. Reported by Basavaraj S m (basavaraj@digiscrypt.com).
Previously, njs_mp_alloc_large() embedded the block descriptor at the end of the allocation for sizes which are not a power of two, so a read past the end of an object stayed inside the malloc()ed region and was not detected by the address sanitizer. The fix is to always allocate the descriptor separately when the address sanitizer is enabled.
github-project-automation
Bot
moved this from New
to In Review
in NGINX OSS Unified Workspace
github-project-automation
Bot
moved this from In Review
to Done
in NGINX OSS Unified Workspace
Merged
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
2 participants
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.
And extended fix for issue reported in #1116.