Skip to content

GH-50893: [C++][Gandiva] fix out-of-bounds read in soundex mappings lookup - #50894

Open
Arawoof06 wants to merge 1 commit into
apache:mainfrom
Arawoof06:soundex-mappings-ascii-bound
Open

GH-50893: [C++][Gandiva] fix out-of-bounds read in soundex mappings lookup#50894
Arawoof06 wants to merge 1 commit into
apache:mainfrom
Arawoof06:soundex-mappings-ascii-bound

Conversation

@Arawoof06

Copy link
Copy Markdown
Contributor

Rationale for this change

soundex_utf8 indexes the 26-entry mappings table with toupper(byte) - 'A', but it classifies letters and folds case with the locale-sensitive isalpha/toupper. Under a non-C locale isalpha accepts bytes 0x80-0xFF and toupper leaves them past 'Z', so the lookup reads past the end of the table. It is reachable from soundex() 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. TestSoundexNonAsciiNoOverread feeds 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.

@github-actions github-actions Bot added the awaiting review Awaiting review label Aug 17, 2026
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50893 has been automatically assigned in GitHub to PR creator.

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