GH-50440: [C++][Gandiva] fix out-of-bounds read in set_error_for_date#50441
Open
Arawoof06 wants to merge 1 commit into
Open
GH-50440: [C++][Gandiva] fix out-of-bounds read in set_error_for_date#50441Arawoof06 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
set_error_for_dateformats the invalid-date/timestamp message withsnprintf(error, size, "%s%s", msg, input), butinputis a Gandiva string passed as pointer pluslengthand is not NUL-terminated, so the%sconversion scans past thelengthbytes looking for a terminator and over-reads the values buffer (past its end for the last value). It is reached fromcastDATE_utf8/castTIMESTAMP_utf8on any invalidCAST(str AS DATE/TIMESTAMP)input. Switching to the bounded%.*sform with the explicit length keeps the read inside the string, matching every other error formatter in the precompiled sources.What changes are included in this PR?
Use
%.*swithlengthinset_error_for_date, which covers all of its call sites in bothcastDATE_utf8andcastTIMESTAMP_utf8.Are these changes tested?
Yes. Added
TestTime.TestCastDateInvalidUnterminated, which feeds an invalid date held in an exactly-sized heap buffer with no trailing NUL so the over-read trips AddressSanitizer, and checks the error string is bounded to the input length.Are there any user-facing changes?
No.
This PR contains a "Critical Fix". It fixes a heap out-of-bounds read reachable from user-supplied CAST-to-date/timestamp input.