OIiveirra commented on code in PR #69526:
URL: https://github.com/apache/doris/pull/69526#discussion_r4365918391
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -471,28 +471,48 @@ const std::shared_ptr<LocalFileSystem>&
global_local_filesystem() {
Status LocalFileSystem::canonicalize_local_file(const std::string& dir,
const std::string& file_path,
std::string* full_path) {
- const std::string absolute_path = dir + "/" + file_path;
+ // An empty root would turn a relative request into an unrestricted
absolute path.
+ if (dir.empty()) {
+ return Status::InvalidArgument("user_files_secure_path must not be
empty");
+ }
+ // Reject embedded NULs before passing paths to filesystem APIs that use C
strings.
+ if (file_path.empty() || file_path.find('\0') != std::string::npos) {
+ return Status::InvalidArgument("local file path must not be empty or
contain NUL");
+ }
+ Path local_path;
+ // Convert file: URIs to filesystem paths and reject remote schemes before
any reader opens them.
+ RETURN_IF_ERROR(convert_to_abs_path(file_path, local_path));
+ std::string secure_path;
+ RETURN_IF_ERROR(canonicalize(dir, &secure_path));
std::string canonical_path;
- RETURN_IF_ERROR(canonicalize(absolute_path, &canonical_path));
- if (!contain_path(dir, canonical_path)) {
- return Status::InvalidArgument("file path is not allowed: {}",
canonical_path);
+ const auto status = canonicalize(Path(secure_path) / local_path,
&canonical_path);
+ // A glob result can reveal external names even when resolution fails on a
dangling symlink.
+ // Reject both failed resolution and out-of-root targets without exposing
expanded names.
+ if (!status.ok() || !contain_path(secure_path, canonical_path)) {
+ return Status::InvalidArgument("file path is not allowed");
}
-
- *full_path = canonical_path;
+ *full_path = std::move(canonical_path);
return Status::OK();
}
Status LocalFileSystem::safe_glob(const std::string& path,
std::vector<FileInfo>* res) {
if (path.find("..") != std::string::npos) {
return Status::InvalidArgument("can not contain '..' in path");
}
- std::string full_path = config::user_files_secure_path + "/" + path;
+ // An empty root would turn the prefix below into "/" and allow any local
file.
+ if (config::user_files_secure_path.empty()) {
+ return Status::InvalidArgument("user_files_secure_path must not be
empty");
+ }
+ std::string secure_path;
+ RETURN_IF_ERROR(canonicalize(config::user_files_secure_path,
&secure_path));
+ std::string full_path = secure_path + "/" + path;
Review Comment:
Fixed in 545e69a9c7a0cfda377caacb24b13e96e0def606. The canonical secure root
is now escaped as a literal glob prefix; only the requested suffix is
interpreted as a pattern. Canonical target containment remains unchanged.
`LocalFileSystemTest.TestSafeGlobSecureRoot` covers a root containing `[]*?`
and a backslash, both directly and through a root symlink. All 4 selected
filesystem tests passed under Clang21/ASAN.
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/LocalTableValuedFunction.java:
##########
@@ -63,6 +64,11 @@ public class LocalTableValuedFunction extends
ExternalFileTableValuedFunction {
private boolean sharedStorage = false;
public LocalTableValuedFunction(Map<String, String> properties) throws
AnalysisException {
+ if (!Config.enable_local_tvf) {
Review Comment:
Fixed in 545e69a9c7a0cfda377caacb24b13e96e0def606. The read constructor and
LOCAL sink binding now share `LocalTableValuedFunction.checkEnabled()`. Sink
binding checks the startup setting before validating properties, so both INSERT
and EXPLAIN INSERT are rejected when disabled.
`BindTVFTableSinkTest` covers disabled requests with complete/incomplete
properties, enabled property validation, and unaffected S3/HDFS sinks. All 15
selected FE tests passed. Groovy assertions were also added; full Groovy
execution remains unverified locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
