GH-50893: [C++][Gandiva] fix out-of-bounds read in soundex mappings lookup - #50894
Open
Arawoof06 wants to merge 1 commit into
Open
GH-50893: [C++][Gandiva] fix out-of-bounds read in soundex mappings lookup#50894Arawoof06 wants to merge 1 commit into
Arawoof06 wants to merge 1 commit into
Conversation
|
|
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.
Rationale for this change
soundex_utf8indexes the 26-entrymappingstable withtoupper(byte) - 'A', but it classifies letters and folds case with the locale-sensitiveisalpha/toupper. Under a non-C localeisalphaaccepts bytes 0x80-0xFF andtoupperleaves them past 'Z', so the lookup reads past the end of the table. It is reachable fromsoundex()on untrusted string values; ASAN reports a global-buffer-overflow read for an input byte such as 0xAA when the locale classifies it as a letter.What changes are included in this PR?
Classify and upper-case with ASCII-only helpers so the table index stays within 0-25. A byte that is not an ASCII letter is treated as a separator, which is what soundex already does for any non-letter.
Are these changes tested?
Yes.
TestSoundexNonAsciiNoOverreadfeeds a value whose non-letter byte is one the active locale reports as a letter; before the change the mappings lookup ran off the end of the table, after it the byte is skipped and the result matches the ASCII-only spelling. The existing soundex cases still pass.Are there any user-facing changes?
No change for ASCII input. A byte above 0x7f is now consistently ignored instead of being folded through a locale-dependent, out-of-range table lookup.
This PR contains a "Critical Fix". The wrong classification lets
soundex()read past the end of the mappings table on an exactly sized input under a non-C locale.