Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2282,17 +2282,10 @@ common::Status NvExecutionProvider::RefitEngine(std::string onnx_model_filename,
"The ONNX model was not provided as path. "
"Please use provide an ONNX bytestream to enable refitting the weightless engine.");
} else {
// check if file path to ONNX is legal
if (path_check && IsAbsolutePath(onnx_model_path.string())) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL,
"For security purpose, the ONNX model path should be set with "
"a relative path, but it is an absolute path: " +
onnx_model_path.string());
}
if (path_check && IsRelativePathToParentPath(onnx_model_path.string())) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL,
"The ONNX model path has '..'. For security purpose, it's not "
"allowed to point outside the directory.");
// Validate that the ONNX model path does not escape the model directory.
if (path_check && !onnx_model_filename.empty()) {
ORT_RETURN_IF_ERROR(utils::ValidateExternalDataPath(
std::filesystem::path(onnx_model_folder_path), std::filesystem::path(onnx_model_filename)));
Comment thread
yuslepukhin marked this conversation as resolved.
Outdated
}

if (!(std::filesystem::exists(onnx_model_path) && std::filesystem::is_regular_file(onnx_model_path))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,18 +350,12 @@ Status TensorRTCacheModelHandler::GetEpContextFromGraph(const Node& node) {
// Get engine from cache file.
std::string cache_path = attrs.at(EP_CACHE_CONTEXT).s();

// For security purpose, in the case of running context model, TRT EP won't allow
// engine cache path to be the relative path like "../file_path" or the absolute path.
// It only allows the engine cache to be in the same directory or sub directory of the context model.
if (IsAbsolutePath(cache_path)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "For security purpose, the ep_cache_context attribute should be set with a relative path, but it is an absolute path: " + cache_path);
}
if (IsRelativePathToParentPath(cache_path)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "The file path in ep_cache_context attribute has '..'. For security purpose, it's not allowed to point outside the directory.");
}
// Validate that the cache path does not escape the model directory.
// Rejects absolute paths, ".." traversal, and symlink-based escapes.
std::filesystem::path ctx_model_dir(GetPathOrParentPathOfCtxModel(ep_context_model_path_));
ORT_RETURN_IF_ERROR(utils::ValidateExternalDataPath(ctx_model_dir, std::filesystem::path(cache_path)));
Comment thread
yuslepukhin marked this conversation as resolved.
Outdated

// The engine cache and context model (current model) should be in the same directory
std::filesystem::path ctx_model_dir(GetPathOrParentPathOfCtxModel(ep_context_model_path_));
auto engine_cache_path = ctx_model_dir.append(cache_path);
LOGS_DEFAULT(VERBOSE) << "[NvTensorRTRTX EP] GetEpContextFromGraph engine_cache_path: " + engine_cache_path.string();

Expand Down
3 changes: 3 additions & 0 deletions onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ std::unique_ptr<ModelBlobWrapper> EPCtxHandler::GetModelBlobStream(const std::fi
if (blob_filepath.empty() && !graph_viewer.ModelPath().empty()) {
blob_filepath = graph_viewer.ModelPath();
}
ORT_THROW_IF_ERROR(utils::ValidateExternalDataPath(blob_filepath, std::filesystem::path(ep_cache_context)));
blob_filepath = blob_filepath.parent_path() / ep_cache_context;
ORT_ENFORCE(std::filesystem::exists(blob_filepath), "Blob file not found: ", blob_filepath.string());
result.reset((std::istream*)new std::ifstream(blob_filepath, std::ios_base::binary | std::ios_base::in));
Expand Down Expand Up @@ -242,6 +243,8 @@ std::shared_ptr<SharedContext> EPCtxHandler::Initialize(const std::vector<IExecu
shared_context->Deserialize(ss);
}
} else {
ORT_THROW_IF_ERROR(utils::ValidateExternalDataPath(session_context.GetOutputModelPath(),
std::filesystem::path(ep_cache_context)));
std::filesystem::path ep_context_path = session_context.GetOutputModelPath().parent_path() / ep_cache_context;
if (ep_context_path.extension() != ".xml") {
shared_context = shared_context_manager_->GetOrCreateSharedContext(ep_context_path);
Expand Down
26 changes: 6 additions & 20 deletions onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,27 +103,13 @@ Status GetEpContextFromMainNode(const onnxruntime::Node& main_context_node,
std::filesystem::path folder_path = std::filesystem::path(ctx_onnx_model_path).parent_path();
std::string external_qnn_ctx_binary_file_name = node_helper.Get(EP_CACHE_CONTEXT, "");
ORT_RETURN_IF(external_qnn_ctx_binary_file_name.empty(), "The file path in ep_cache_context should not be empty.");
#ifdef _WIN32
onnxruntime::PathString external_qnn_context_binary_path = onnxruntime::ToPathString(external_qnn_ctx_binary_file_name);
auto ctx_file_path = std::filesystem::path(external_qnn_context_binary_path.c_str());
ORT_RETURN_IF(ctx_file_path.is_absolute(), "External mode should set ep_cache_context field with a relative path, but it is an absolute path: ",
external_qnn_ctx_binary_file_name);
auto relative_path = ctx_file_path.lexically_normal().make_preferred().wstring();
if (relative_path.find(L"..", 0) != std::string::npos) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH, "The file path in ep_cache_context field has '..'. It's not allowed to point outside the directory.");
}

std::filesystem::path context_binary_path = folder_path.append(relative_path);
#else
ORT_RETURN_IF(external_qnn_ctx_binary_file_name[0] == '/',
"External mode should set ep_cache_context field with a relative path, but it is an absolute path: ",
external_qnn_ctx_binary_file_name);
if (external_qnn_ctx_binary_file_name.find("..", 0) != std::string::npos) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH, "The file path in ep_cache_context field has '..'. It's not allowed to point outside the directory.");
}
std::filesystem::path context_binary_path = folder_path.append(external_qnn_ctx_binary_file_name);
std::string file_full_path = context_binary_path.string();
#endif
// Validate that the cache path does not escape the model directory.
// Rejects absolute paths, ".." traversal, and symlink-based escapes.
ORT_RETURN_IF_ERROR(utils::ValidateExternalDataPath(
std::filesystem::path(ctx_onnx_model_path), std::filesystem::path(external_qnn_ctx_binary_file_name)));
Comment thread
yuslepukhin marked this conversation as resolved.
Outdated

std::filesystem::path context_binary_path = folder_path / external_qnn_ctx_binary_file_name;
if (!std::filesystem::is_regular_file(context_binary_path)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH, "The file path in ep_cache_context does not exist or is not accessible.");
}
Expand Down
8 changes: 7 additions & 1 deletion onnxruntime/core/providers/qnn/qnn_execution_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1007,8 +1007,14 @@ QNNExecutionProvider::GetCapability(const onnxruntime::GraphViewer& graph_viewer

for (auto& ep_ctx_node : ep_ctx_nodes) {
NodeAttrHelper node_helper(*ep_ctx_node);
std::string ep_cache_context_value = node_helper.Get(qnn::EP_CACHE_CONTEXT, "");
if (ep_cache_context_value.empty()) {
Comment thread
yuslepukhin marked this conversation as resolved.
continue;
}
ORT_THROW_IF_ERROR(utils::ValidateExternalDataPath(
std::filesystem::path(context_model_path), std::filesystem::path(ep_cache_context_value)));
std::string context_bin_filepath(parent_path.string());
context_bin_filepath.append("/").append(node_helper.Get(qnn::EP_CACHE_CONTEXT, ""));
context_bin_filepath.append("/").append(ep_cache_context_value);
if (context_bin_map.find(context_bin_filepath) == context_bin_map.end()) {
context_bin_map.emplace(context_bin_filepath, std::make_unique<std::vector<std::string>>());
// Push context bin filepath for lookup between sessions
Expand Down
5 changes: 5 additions & 0 deletions onnxruntime/core/providers/shared_library/provider_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,11 @@ inline bool HasExternalDataInMemory(const ONNX_NAMESPACE::TensorProto& ten_proto
return g_host->Utils__HasExternalDataInMemory(ten_proto);
}

inline Status ValidateExternalDataPath(const std::filesystem::path& model_path,
const std::filesystem::path& external_data_path) {
return g_host->Utils__ValidateExternalDataPath(model_path, external_data_path);
}

} // namespace utils

namespace graph_utils {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,9 @@ struct ProviderHost {

virtual bool Utils__HasExternalDataInMemory(const ONNX_NAMESPACE::TensorProto& ten_proto) = 0;

virtual Status Utils__ValidateExternalDataPath(const std::filesystem::path& model_path,
const std::filesystem::path& external_data_path) = 0;

// Model
virtual std::unique_ptr<Model> Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path,
const IOnnxRuntimeOpSchemaRegistryList* local_registries,
Expand Down
14 changes: 4 additions & 10 deletions onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,18 +323,12 @@ Status TensorRTCacheModelHandler::GetEpContextFromGraph(const GraphViewer& graph
// Get engine from cache file.
std::string cache_path = attrs.at(EP_CACHE_CONTEXT).s();

// For security purpose, in the case of running context model, TRT EP won't allow
// engine cache path to be the relative path like "../file_path" or the absolute path.
// It only allows the engine cache to be in the same directory or sub directory of the context model.
if (IsAbsolutePath(cache_path)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "For security purpose, the ep_cache_context attribute should be set with a relative path, but it is an absolute path: " + cache_path);
}
if (IsRelativePathToParentPath(cache_path)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "The file path in ep_cache_context attribute has '..'. For security purpose, it's not allowed to point outside the directory.");
}
// Validate that the cache path does not escape the model directory.
// Rejects absolute paths, ".." traversal, and symlink-based escapes.
std::filesystem::path ctx_model_dir(GetPathOrParentPathOfCtxModel(ep_context_model_path_));
ORT_RETURN_IF_ERROR(utils::ValidateExternalDataPath(ctx_model_dir, std::filesystem::path(cache_path)));
Comment thread
yuslepukhin marked this conversation as resolved.
Outdated

// The engine cache and context model (current model) should be in the same directory
std::filesystem::path ctx_model_dir(GetPathOrParentPathOfCtxModel(ep_context_model_path_));
auto engine_cache_path = ctx_model_dir.append(cache_path);
LOGS_DEFAULT(VERBOSE) << "[TensorRT EP] GetEpContextFromGraph engine_cache_path: " + engine_cache_path.string();

Expand Down
15 changes: 4 additions & 11 deletions onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2895,17 +2895,10 @@ common::Status TensorrtExecutionProvider::RefitEngine(std::string onnx_model_fil
"Please use provide an ONNX bytestream to enable refitting the weightless engine."
"When providing a bytestream during session initialization, it should also be set as trt_onnx_bytes_stream");
} else {
// check if file path to ONNX is legal
if (path_check && IsAbsolutePath(onnx_model_path.string())) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL,
"For security purpose, the ONNX model path should be set with "
"a relative path, but it is an absolute path: " +
onnx_model_path.string());
}
if (path_check && IsRelativePathToParentPath(onnx_model_path.string())) {
return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL,
"The ONNX model path has '..'. For security purpose, it's not "
"allowed to point outside the directory.");
// Validate that the ONNX model path does not escape the model directory.
if (path_check && !onnx_model_filename.empty()) {
ORT_RETURN_IF_ERROR(utils::ValidateExternalDataPath(
std::filesystem::path(onnx_model_folder_path), std::filesystem::path(onnx_model_filename)));
Comment thread
yuslepukhin marked this conversation as resolved.
Outdated
}

if (!(std::filesystem::exists(onnx_model_path) && std::filesystem::is_regular_file(onnx_model_path))) {
Expand Down
5 changes: 5 additions & 0 deletions onnxruntime/core/session/provider_bridge_ort.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,11 @@ struct ProviderHostImpl : ProviderHost {
return onnxruntime::utils::HasExternalDataInMemory(ten_proto);
}

Status Utils__ValidateExternalDataPath(const std::filesystem::path& model_path,
const std::filesystem::path& external_data_path) override {
return onnxruntime::utils::ValidateExternalDataPath(model_path, external_data_path);
}

// Model (wrapped)
std::unique_ptr<Model> Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path,
const IOnnxRuntimeOpSchemaRegistryList* local_registries,
Expand Down
Loading