diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index e9775fe23fe08..8685655420a38 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -402,39 +402,51 @@ static bool HasPathComponentPrefix(const std::filesystem::path& prefix, const st return prefix_end == prefix.end(); } -Status ValidateExternalDataPath(const std::filesystem::path& model_path, - const std::filesystem::path& external_data_path) { +/// Validates that `external_data_path` is a relative path contained within `model_dir` after symlink resolution, +/// and that the resolved file exists. This is the core security check for EP context cache paths. +/// +/// Validation steps: +/// 1. Reject empty paths +/// 2. Reject absolute paths (including Unix-style '/...' on Windows) +/// 3. Skip remaining checks on WASM if no filesystem is available +/// 4. Resolve `model_dir / external_data_path` to a canonical path (resolving symlinks for existing segments) +/// 5. Verify the canonical path is a prefix-child of the canonical model_dir (containment check) +/// 6. Verify the resolved file exists on disk +/// +/// This function does NOT handle the symlinked-model fallback — that is the responsibility of +/// ValidateExternalDataPath(), which calls this function as a first pass. +Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir, + const std::filesystem::path& external_data_path) { + // Step 1: Reject empty external data paths. ORT_RETURN_IF(external_data_path.empty(), "Empty external data path not allowed"); - // Note: Use !root_path().empty() to reject paths like '/some/path` even on Windows. + // Step 2: Reject absolute paths. + // Use !root_path().empty() to reject paths like '/some/path' even on Windows (where is_absolute() + // requires a drive letter). ORT_RETURN_IF(!external_data_path.root_path().empty(), "Absolute path not allowed for external data location"); #if defined(__wasm__) + // Step 3 (WASM only): If we can't access the current working directory, assume the WASM environment + // does not have a virtual filesystem and defer validation to an ExternalDataLoader for the WASM EP. std::error_code error_code; std::filesystem::current_path(error_code); if (error_code) { - // If we can't access the current working directory in a WASM build, we assume that the WASM - // environment does not have a virtual filesystem and defer validation to an ExternalDataLoader for - // a WASM EP. return Status::OK(); } #endif - // Determine the model directory: use model file's parent directory if provided, - // otherwise use the current working directory. - std::filesystem::path model_dir = model_path.empty() || model_path.parent_path().empty() - ? std::filesystem::path{"."} - : model_path.parent_path(); + // Step 4: Resolve both the model directory and the combined path to canonical forms. + // WeaklyCanonicalPath resolves symlinks for existing path segments while lexically normalizing + // non-existent trailing segments. + std::filesystem::path resolved_dir = model_dir.empty() ? std::filesystem::path{"."} : model_dir; - // Resolve the model directory and the external data path to their weakly canonical forms, which - // resolves symlinks but does not require that the paths actually exist yet. std::filesystem::path model_dir_canonical; std::filesystem::path external_data_path_canonical; - ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(model_dir, model_dir_canonical)); + ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(resolved_dir, model_dir_canonical)); ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(model_dir_canonical / external_data_path, external_data_path_canonical)); - // Check that the external data path is contained by the model directory. - // If it is, check if the external data file actually exists. + // Step 5: Containment check — verify the resolved external data path starts with the model directory. + // Step 6: Existence check — verify the file actually exists on disk. if (HasPathComponentPrefix(model_dir_canonical, external_data_path_canonical)) { bool path_exists = false; ORT_RETURN_IF_ERROR(PathExists(external_data_path_canonical, path_exists)); @@ -442,50 +454,108 @@ Status ValidateExternalDataPath(const std::filesystem::path& model_path, return Status::OK(); } + return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, + "External data path escapes model directory. ", + "External data path: ", external_data_path, " resolved path: ", + external_data_path_canonical, " ", "allowed directory: ", resolved_dir); +} + +/// Validates that `external_data_path` is a safe relative path under the model's directory. +/// This is the primary entry point for validating external data locations when loading ONNX models. +/// +/// Validation flow: +/// 1. Try ValidateExternalDataPathFromDir against the model file's parent directory. +/// If it passes, return success. +/// 2. If it fails due to empty/absolute external_data_path, return the error immediately +/// (these are input errors unrelated to the model location). +/// 3. If model_path is empty (model loaded from bytes), wrap the error with context. +/// 4. If model_path is a symlink, try the symlink fallback: +/// - Resolve the external data path from the *symlink* model directory to its canonical form. +/// - Check if that canonical target is under the *real* (resolved) model directory. +/// This supports Hugging Face Hub local cache layouts where both the model file and +/// external data files are symlinks into a shared blob store: +/// snapshots/v1/model.onnx -> ../../blobs/sha256-abc (model symlink) +/// snapshots/v1/data.bin -> ../../blobs/sha256-def (data symlink) +/// Both symlink targets live under blobs/, which is the real model directory. +/// 5. If none of the above succeed, return an error indicating directory escape. +Status ValidateExternalDataPath(const std::filesystem::path& model_path, + const std::filesystem::path& external_data_path) { + // Derive the model directory from the model file path. + // If model_path is empty (loaded from bytes) or has no directory component (bare filename), + // fall back to "." (current working directory). + std::filesystem::path model_dir = model_path.empty() || model_path.parent_path().empty() + ? std::filesystem::path{"."} + : model_path.parent_path(); + + // --- Pass 1: Validate against the model file's parent directory --- + Status status = ValidateExternalDataPathFromDir(model_dir, external_data_path); + if (status.IsOK()) { + return status; + } + + // --- Guard: Don't retry for input-validation errors --- + // Empty and absolute paths are always invalid regardless of model directory or symlinks. + // Return the error directly without misleading "escapes directory" context. + if (external_data_path.empty() || !external_data_path.root_path().empty()) { + return status; + } + + // --- Empty model_path: model loaded from bytes --- + // When there's no model file path, the working directory is used as the base. + // Provide a specific error message indicating this context. if (model_path.empty()) { return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "External data path for model loaded from bytes escapes working directory. ", - "External data path: ", external_data_path, " resolved path: ", - external_data_path_canonical, " ", "working directory: ", model_dir); + status.ErrorMessage()); } - // The model file itself may be a symlink. Therefore, check against the real/canonical directory of the model - // after resolving all symlinks. + // --- Pass 2: Symlink fallback for symlinked model files --- // - // This supports symlinked models (e.g., Hugging Face Hub local cache) where the canonical - // parent of the model file differs from the parent directory of the symlinked model file. + // When model_path is a symlink, the model's *apparent* parent directory (where the symlink lives) + // differs from its *real* parent directory (where the symlink target lives). External data files + // may also be symlinks in the apparent directory that resolve to the real directory tree. + // + // Example (Hugging Face Hub cache): + // apparent dir: ~/.cache/huggingface/hub/models--foo/snapshots/abc123/ + // real dir: ~/.cache/huggingface/hub/models--foo/blobs/ + // model.onnx -> ../../blobs/sha256-111 (symlink) + // weights.bin -> ../../blobs/sha256-222 (symlink) + // + // Pass 1 fails because "weights.bin" resolved from apparent_dir points to blobs/sha256-222, + // which is not under apparent_dir. This fallback checks if it's under real_dir instead. std::error_code ec; if (!std::filesystem::is_symlink(model_path, ec)) { - // Note: is_symlink returns false if file is not a symlink, file does not exist, or an error - // occurred (e.g., permissions). In any of these cases, we just return an error. - std::string fs_error_msg; - if (ec) { - fs_error_msg = " filesystem::is_symlink error: " + ec.message(); - } - - return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, - "External data path for model escapes model directory. ", - "External data path: ", external_data_path, " resolved path: ", - external_data_path_canonical, " ", "model directory: ", model_dir, fs_error_msg); + // model_path is not a symlink (or doesn't exist, or we lack permissions). + // No fallback possible — return the original containment error. + return status; } + // Resolve the model symlink to get the real model directory. std::filesystem::path real_model_path; ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(model_path, real_model_path)); auto real_model_dir = real_model_path.parent_path(); - // Check that the external data path is contained by the real model directory. - // If it is, check if the external data file actually exists. - if (HasPathComponentPrefix(real_model_dir, external_data_path_canonical)) { + // Resolve the external data path from the *apparent* (symlink) model directory. + // This follows any symlinks in the external data file itself. + std::filesystem::path external_data_full_path = model_dir / external_data_path; + std::filesystem::path external_data_canonical; + ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(external_data_full_path, external_data_canonical)); + + // Check if the resolved external data target is under the real model directory. + std::filesystem::path real_model_dir_canonical; + ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(real_model_dir, real_model_dir_canonical)); + + if (HasPathComponentPrefix(real_model_dir_canonical, external_data_canonical)) { bool path_exists = false; - ORT_RETURN_IF_ERROR(PathExists(external_data_path_canonical, path_exists)); - ORT_RETURN_IF(!path_exists, "External data path does not exist: ", external_data_path_canonical); + ORT_RETURN_IF_ERROR(PathExists(external_data_canonical, path_exists)); + ORT_RETURN_IF(!path_exists, "External data path does not exist: ", external_data_canonical); return Status::OK(); } return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, - "External data path: ", external_data_path, " (resolved path: ", - external_data_path_canonical, ") escapes both model directory: ", model_dir, - " and real model directory: ", real_model_dir); + "External data path escapes model directory. ", + "External data path: ", external_data_path, " resolved path: ", + external_data_canonical, " ", "allowed directory: ", real_model_dir); } Status GetExternalDataInfo(const ONNX_NAMESPACE::TensorProto& tensor_proto, diff --git a/onnxruntime/core/framework/tensorprotoutils.h b/onnxruntime/core/framework/tensorprotoutils.h index 22e39e7ae16ed..63f7b5e78e478 100644 --- a/onnxruntime/core/framework/tensorprotoutils.h +++ b/onnxruntime/core/framework/tensorprotoutils.h @@ -589,6 +589,19 @@ Status TensorProtoWithExternalDataToTensorProto( Status ValidateExternalDataPath(const std::filesystem::path& model_path, const std::filesystem::path& external_data_path); +/// +/// Validates that the given external data path is not an absolute path, is under the given directory +/// (after resolving symlinks), and exists. +/// +/// Use this overload when you already have the directory that should contain the external data +/// (e.g., EP context model directories). Use ValidateExternalDataPath() when you have a model file path. +/// +/// Directory that should contain the external data. Falls back to "." if empty. +/// External data file path to be validated (must be relative). +/// The function will fail if the resolved `external_data_path` path is not under model_dir +Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir, + const std::filesystem::path& external_data_path); + #endif // !defined(SHARED_PROVIDER) inline bool HasType(const ONNX_NAMESPACE::AttributeProto& at_proto) { diff --git a/onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc b/onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc index a07fa5b5ab395..3dcd0695d4e12 100644 --- a/onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc +++ b/onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc @@ -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::ValidateExternalDataPathFromDir( + std::filesystem::path(onnx_model_folder_path), std::filesystem::path(onnx_model_filename))); } if (!(std::filesystem::exists(onnx_model_path) && std::filesystem::is_regular_file(onnx_model_path))) { diff --git a/onnxruntime/core/providers/nv_tensorrt_rtx/onnx_ctx_model_helper.cc b/onnxruntime/core/providers/nv_tensorrt_rtx/onnx_ctx_model_helper.cc index 0b137c4674b00..e8d40352979cd 100644 --- a/onnxruntime/core/providers/nv_tensorrt_rtx/onnx_ctx_model_helper.cc +++ b/onnxruntime/core/providers/nv_tensorrt_rtx/onnx_ctx_model_helper.cc @@ -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::ValidateExternalDataPathFromDir(ctx_model_dir, std::filesystem::path(cache_path))); // 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(); diff --git a/onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc b/onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc index 80ce4de4a6a19..a21916be4c59b 100644 --- a/onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc +++ b/onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc @@ -114,6 +114,7 @@ std::unique_ptr 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)); @@ -242,6 +243,8 @@ std::shared_ptr EPCtxHandler::Initialize(const std::vectorDeserialize(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); diff --git a/onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc b/onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc index 68aa9a157f4a2..3d2f167653473 100644 --- a/onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc +++ b/onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc @@ -103,27 +103,16 @@ 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."); + // Validate that the cache path does not escape the model directory. + // Rejects absolute paths, ".." traversal, and symlink-based escapes. + auto validate_status = ::onnxruntime::utils::ValidateExternalDataPath( + std::filesystem::path(ctx_onnx_model_path), std::filesystem::path(external_qnn_ctx_binary_file_name)); + if (!validate_status.IsOK()) { + return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH, validate_status.ErrorMessage()); } - 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 + + 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."); } diff --git a/onnxruntime/core/providers/qnn/qnn_execution_provider.cc b/onnxruntime/core/providers/qnn/qnn_execution_provider.cc index 6be50ac4da5c3..27f0183853955 100644 --- a/onnxruntime/core/providers/qnn/qnn_execution_provider.cc +++ b/onnxruntime/core/providers/qnn/qnn_execution_provider.cc @@ -1007,8 +1007,18 @@ 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()) { + continue; + } + auto validate_status = utils::ValidateExternalDataPath( + std::filesystem::path(context_model_path), std::filesystem::path(ep_cache_context_value)); + if (!validate_status.IsOK()) { + LOGS(logger, ERROR) << "QNN EP context path validation failed: " << validate_status.ErrorMessage(); + return result; + } 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>()); // Push context bin filepath for lookup between sessions diff --git a/onnxruntime/core/providers/shared_library/provider_api.h b/onnxruntime/core/providers/shared_library/provider_api.h index 39cba15ca3ddc..a13938922bc54 100644 --- a/onnxruntime/core/providers/shared_library/provider_api.h +++ b/onnxruntime/core/providers/shared_library/provider_api.h @@ -462,6 +462,16 @@ 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); +} + +inline Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir, + const std::filesystem::path& external_data_path) { + return g_host->Utils__ValidateExternalDataPathFromDir(model_dir, external_data_path); +} + } // namespace utils namespace graph_utils { diff --git a/onnxruntime/core/providers/shared_library/provider_interfaces.h b/onnxruntime/core/providers/shared_library/provider_interfaces.h index 720b180f4d589..1faaba653fc51 100644 --- a/onnxruntime/core/providers/shared_library/provider_interfaces.h +++ b/onnxruntime/core/providers/shared_library/provider_interfaces.h @@ -1004,6 +1004,11 @@ 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; + virtual Status Utils__ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir, + const std::filesystem::path& external_data_path) = 0; + // Model virtual std::unique_ptr Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path, const IOnnxRuntimeOpSchemaRegistryList* local_registries, diff --git a/onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc b/onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc index 091b110d8c746..7c8b5f5890073 100644 --- a/onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc +++ b/onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc @@ -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::ValidateExternalDataPathFromDir(ctx_model_dir, std::filesystem::path(cache_path))); // 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(); diff --git a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc index 00c7815814f5b..52fcbf088c215 100644 --- a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc +++ b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc @@ -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::ValidateExternalDataPathFromDir( + std::filesystem::path(onnx_model_folder_path), std::filesystem::path(onnx_model_filename))); } if (!(std::filesystem::exists(onnx_model_path) && std::filesystem::is_regular_file(onnx_model_path))) { diff --git a/onnxruntime/core/session/provider_bridge_ort.cc b/onnxruntime/core/session/provider_bridge_ort.cc index 403d6c4050310..c45d21639c008 100644 --- a/onnxruntime/core/session/provider_bridge_ort.cc +++ b/onnxruntime/core/session/provider_bridge_ort.cc @@ -1295,6 +1295,16 @@ 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); + } + + Status Utils__ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir, + const std::filesystem::path& external_data_path) override { + return onnxruntime::utils::ValidateExternalDataPathFromDir(model_dir, external_data_path); + } + // Model (wrapped) std::unique_ptr Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path, const IOnnxRuntimeOpSchemaRegistryList* local_registries, diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index fa34e9722b66b..38a0f15f2301d 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -716,6 +716,164 @@ TEST_F(PathValidationTest, ValidateExternalDataPathEmptyModelPathWithSymlinkOuts EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("escapes working directory")); } +// Test that symlinked model + symlinked data in same blob store passes (HuggingFace layout). +// Layout: +// base_dir_/blobs/model_blob (real model file) +// base_dir_/blobs/data_blob (real data file) +// base_dir_/snapshots/v1/model.onnx -> ../../blobs/model_blob (symlink) +// base_dir_/snapshots/v1/data.bin -> ../../blobs/data_blob (symlink) +TEST_F(PathValidationTest, ValidateExternalDataPathSymlinkedModelAndData_HuggingFaceLayout) { + auto blobs_dir = base_dir_ / "blobs"; + auto snapshots_dir = base_dir_ / "snapshots" / "v1"; + try { + CreateDirectories(blobs_dir); + CreateDirectories(snapshots_dir); + + // Create real files in blobs/ + std::ofstream{blobs_dir / "model_blob"}; + std::ofstream{blobs_dir / "data_blob"}; + + // Create symlinks in snapshots/v1/ + std::filesystem::create_symlink(blobs_dir / "model_blob", snapshots_dir / "model.onnx"); + std::filesystem::create_symlink(blobs_dir / "data_blob", snapshots_dir / "data.bin"); + } catch (const std::exception& e) { + GTEST_SKIP() << "Skipping symlink test: " << e.what(); + } + + // model.onnx is a symlink; data.bin is also a symlink. + // Both resolve to the same blobs/ directory — should pass. + ASSERT_STATUS_OK(utils::ValidateExternalDataPath(snapshots_dir / "model.onnx", "data.bin")); +} + +// Test that symlinked model + empty external data path is rejected (not silently accepted). +TEST_F(PathValidationTest, ValidateExternalDataPathSymlinkedModel_EmptyPathRejected) { + auto blobs_dir = base_dir_ / "blobs"; + auto snapshots_dir = base_dir_ / "snapshots" / "v1"; + try { + CreateDirectories(blobs_dir); + CreateDirectories(snapshots_dir); + std::ofstream{blobs_dir / "model_blob"}; + std::filesystem::create_symlink(blobs_dir / "model_blob", snapshots_dir / "model.onnx"); + } catch (const std::exception& e) { + GTEST_SKIP() << "Skipping symlink test: " << e.what(); + } + + Status status = utils::ValidateExternalDataPath(snapshots_dir / "model.onnx", ""); + ASSERT_FALSE(status.IsOK()); + EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("Empty external data path")); +} + +// Test that symlinked model + absolute external data path is rejected without symlink fallback. +TEST_F(PathValidationTest, ValidateExternalDataPathSymlinkedModel_AbsolutePathRejected) { + auto blobs_dir = base_dir_ / "blobs"; + auto snapshots_dir = base_dir_ / "snapshots" / "v1"; + try { + CreateDirectories(blobs_dir); + CreateDirectories(snapshots_dir); + std::ofstream{blobs_dir / "model_blob"}; + std::filesystem::create_symlink(blobs_dir / "model_blob", snapshots_dir / "model.onnx"); + } catch (const std::exception& e) { + GTEST_SKIP() << "Skipping symlink test: " << e.what(); + } + + Status status = utils::ValidateExternalDataPath(snapshots_dir / "model.onnx", "/etc/passwd"); + ASSERT_FALSE(status.IsOK()); + EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("Absolute path not allowed")); +} + +// Test that symlinked model + data symlink resolving OUTSIDE the real model dir is rejected. +TEST_F(PathValidationTest, ValidateExternalDataPathSymlinkedModel_DataEscapesRealDir) { + auto blobs_dir = base_dir_ / "blobs"; + auto snapshots_dir = base_dir_ / "snapshots" / "v1"; + try { + CreateDirectories(blobs_dir); + CreateDirectories(snapshots_dir); + std::ofstream{blobs_dir / "model_blob"}; + std::filesystem::create_symlink(blobs_dir / "model_blob", snapshots_dir / "model.onnx"); + + // Create data symlink that resolves outside of blobs/ (the real model dir) + auto outside_target = outside_dir_ / "evil.bin"; + std::ofstream{outside_target}; + std::filesystem::create_symlink(outside_target, snapshots_dir / "evil_data.bin"); + } catch (const std::exception& e) { + GTEST_SKIP() << "Skipping symlink test: " << e.what(); + } + + Status status = utils::ValidateExternalDataPath(snapshots_dir / "model.onnx", "evil_data.bin"); + ASSERT_FALSE(status.IsOK()); + EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("escapes model directory")); +} + +// Test that ValidateExternalDataPathFromDir rejects a non-existent file even if path is valid. +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_NonExistentFileRejected) { + // base_dir_ exists but "no_such_file.bin" does not. + Status status = utils::ValidateExternalDataPathFromDir(base_dir_, "no_such_file.bin"); + ASSERT_FALSE(status.IsOK()); + EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("does not exist")); +} + +// Tests for ValidateExternalDataPathFromDir (directory-based overload used by EPs). +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_ValidSubpath) { + // A valid relative path under the given directory should pass. + CreateEmptyFile(base_dir_ / "engine.cache"); + CreateDirectories(base_dir_ / "subdir"); + CreateEmptyFile(base_dir_ / "subdir" / "engine.cache"); + + ASSERT_STATUS_OK(utils::ValidateExternalDataPathFromDir(base_dir_, "engine.cache")); + ASSERT_STATUS_OK(utils::ValidateExternalDataPathFromDir(base_dir_, "subdir/engine.cache")); +#ifdef _WIN32 + ASSERT_STATUS_OK(utils::ValidateExternalDataPathFromDir(base_dir_, "subdir\\engine.cache")); +#endif +} + +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_EscapeViaDotDot) { + // A path with ".." that escapes the directory must fail. + CreateEmptyFile(outside_dir_ / "data.bin"); + + Status status = utils::ValidateExternalDataPathFromDir(base_dir_, "../data.bin"); + ASSERT_FALSE(status.IsOK()); + EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("External data path escapes model directory")); +} + +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_AbsolutePathRejected) { + // Absolute paths must be rejected. +#ifdef _WIN32 + Status status = utils::ValidateExternalDataPathFromDir(base_dir_, "C:\\data.bin"); + ASSERT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); +#endif + Status status_unix = utils::ValidateExternalDataPathFromDir(base_dir_, "/data.bin"); + ASSERT_THAT(status_unix.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); +} + +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_EmptyPathRejected) { + Status status = utils::ValidateExternalDataPathFromDir(base_dir_, ""); + ASSERT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Empty external data path")); +} + +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_EmptyDirFallsToCwd) { + // When directory is empty, should fall back to current working directory. + std::filesystem::path cwd = std::filesystem::current_path(); + CreateEmptyFile(cwd / "fromdir_test_data.bin"); + other_files_.push_back(cwd / "fromdir_test_data.bin"); + + ASSERT_STATUS_OK(utils::ValidateExternalDataPathFromDir("", "fromdir_test_data.bin")); +} + +TEST_F(PathValidationTest, ValidateExternalDataPathFromDir_SymlinkOutsideRejected) { + // A symlink that resolves outside the given directory must be rejected. + auto outside_target = outside_dir_ / "outside_engine.bin"; + try { + std::ofstream{outside_target}; + auto symlink_path = base_dir_ / "escape_link.bin"; + std::filesystem::create_symlink(outside_target, symlink_path); + } catch (const std::exception& e) { + GTEST_SKIP() << "Skipping symlink test: " << e.what(); + } + + Status status = utils::ValidateExternalDataPathFromDir(base_dir_, "escape_link.bin"); + ASSERT_FALSE(status.IsOK()); +} + #if defined(_WIN32) // Direct tests for the Windows AppContainer fallback used by // WindowsEnv::GetWeaklyCanonicalPath. The AppContainer trigger itself can't be