This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 6cd5e7efc2a GH-51005: [C++] Fix over-read in UriFromAbsolutePath posix
branch (#51006)
6cd5e7efc2a is described below
commit 6cd5e7efc2a5f9f73385c162dd59d1f442e134b4
Author: Abdul Rawoof Khan <[email protected]>
AuthorDate: Wed Aug 26 15:35:30 2026 +0530
GH-51005: [C++] Fix over-read in UriFromAbsolutePath posix branch (#51006)
### Rationale for this change
the posix branch of `UriFromAbsolutePath` passes `path.data()` from a
`std::string_view` straight into `uriUnixFilenameToUriStringA`, which scans its
argument as a nul-terminated c string. a `string_view` is 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 because `out` is sized from `path.length()`
a longer run also writes past `out`. the windows branch just above already
sidesteps this by copying int [...]
### What changes are included in this PR?
copy the view into a `std::string` on the posix branch before the call and
size the output from that copy, mirroring the windows branch.
### Are these changes tested?
yes. `UriFromAbsolutePath.NonNulTerminatedView` passes a prefix view of a
longer buffer; before the change the conversion consumed the trailing bytes and
returned `file:///tmp/foo%20and%20more%20b...`, after it returns
`file:///tmp/foo`. the existing `UriFromAbsolutePath.Basics` still 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 `UriFromAbsolutePath` entry
point when the caller passes a view that is not nul-terminated.
* GitHub Issue: #51005
Authored-by: abdul rawoof <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/util/uri.cc | 9 +++++++--
cpp/src/arrow/util/uri_test.cc | 16 ++++++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/cpp/src/arrow/util/uri.cc b/cpp/src/arrow/util/uri.cc
index 6c0787a87e0..25af8a74c38 100644
--- a/cpp/src/arrow/util/uri.cc
+++ b/cpp/src/arrow/util/uri.cc
@@ -338,8 +338,13 @@ Result<std::string> UriFromAbsolutePath(std::string_view
path) {
// uriWindowsFilenameToUriStringA basically only fails if a null pointer is
given.
ARROW_CHECK_EQ(r, 0) << "uriWindowsFilenameToUriStringA unexpectedly failed";
#else
- out.resize(7 + 3 * path.length() + 1);
- int r = uriUnixFilenameToUriStringA(path.data(), out.data());
+ // uriUnixFilenameToUriStringA scans its argument as a NUL-terminated C
string,
+ // but a std::string_view is not required to be NUL-terminated. Copy into a
+ // std::string first (as the Windows branch above already does) so the
routine
+ // cannot read past the end of the view.
+ std::string fixed_path(path);
+ out.resize(7 + 3 * fixed_path.length() + 1);
+ int r = uriUnixFilenameToUriStringA(fixed_path.data(), out.data());
// same as above (uriWindowsFilenameToUriStringA)
ARROW_CHECK_EQ(r, 0) << "uriUnixFilenameToUriStringA unexpectedly failed";
#endif
diff --git a/cpp/src/arrow/util/uri_test.cc b/cpp/src/arrow/util/uri_test.cc
index 36e09b1b2e8..8c9f932f54e 100644
--- a/cpp/src/arrow/util/uri_test.cc
+++ b/cpp/src/arrow/util/uri_test.cc
@@ -17,6 +17,7 @@
#include <memory>
#include <string>
+#include <string_view>
#include <utility>
#include <vector>
@@ -370,4 +371,19 @@ TEST(UriFromAbsolutePath, Basics) {
#endif
}
+TEST(UriFromAbsolutePath, NonNulTerminatedView) {
+ // The argument is a std::string_view, which is not required to be
+ // NUL-terminated. A view backed by a larger buffer must not make the
+ // conversion consume bytes beyond the view's length.
+#ifdef _WIN32
+ std::string backing = "C:/foo/bar and more bytes";
+ std::string_view path(backing.data(), std::string_view("C:/foo/bar").size());
+ ASSERT_OK_AND_EQ("file:///C:/foo/bar", UriFromAbsolutePath(path));
+#else
+ std::string backing = "/tmp/foo and more bytes";
+ std::string_view path(backing.data(), std::string_view("/tmp/foo").size());
+ ASSERT_OK_AND_EQ("file:///tmp/foo", UriFromAbsolutePath(path));
+#endif
+}
+
} // namespace arrow::util
