GH-51005: [C++] Fix over-read in UriFromAbsolutePath posix branch - #51006
Merged
Conversation
the posix branch of UriFromAbsolutePath hands path.data() from a std::string_view straight to uriUnixFilenameToUriStringA, which scans its argument as a nul-terminated c string. a view is not required to be nul-terminated, so one backed by a larger buffer makes the routine read past the view, and because out was sized from path.length() a longer run also writes past out. the windows branch above already sidesteps this by copying into a std::string first, so do the same on posix and pass the terminated buffer.
|
|
pitrou
changed the title
pitrou
approved these changes
Aug 26, 2026
pitrou
left a comment
pitrou
left a comment
Member
There was a problem hiding this comment.
Nice catch @Arawoof06 . Thanks a lot for fixing this!
github-actions
Bot
added
awaiting committer review
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.
Rationale for this change
the posix branch of
UriFromAbsolutePathpassespath.data()from astd::string_viewstraight intouriUnixFilenameToUriStringA, which scans its argument as a nul-terminated c string. astring_viewis not required to be nul-terminated, so a view backed by a larger buffer makes the vendored routine read past the end of the view, and becauseoutis sized frompath.length()a longer run also writes pastout. the windows branch just above already sidesteps this by copying into astd::stringfirst.What changes are included in this PR?
copy the view into a
std::stringon the posix branch before the call and size the output from that copy, mirroring the windows branch.Are these changes tested?
yes.
UriFromAbsolutePath.NonNulTerminatedViewpasses a prefix view of a longer buffer; before the change the conversion consumed the trailing bytes and returnedfile:///tmp/foo%20and%20more%20b..., after it returnsfile:///tmp/foo. the existingUriFromAbsolutePath.Basicsstill passes.Are there any user-facing changes?
no.
This PR contains a "Critical Fix". it fixes a heap out-of-bounds read (and possible write) reachable through the public
UriFromAbsolutePathentry point when the caller passes a view that is not nul-terminated.