Cleanup external data path validation

onnxruntime/core/framework/tensorprotoutils.cc

@@ -349,66 +349,84 @@ Status TensorProtoWithExternalDataToTensorProto(
   return Status::OK();
 }
 
-Status ValidateExternalDataPath(const std::filesystem::path& base_dir,
-                                const std::filesystem::path& location,
-                                const std::filesystem::path& model_path) {
-  // Reject absolute paths
-  ORT_RETURN_IF(location.is_absolute(),
-                "Absolute paths not allowed for external data location");
-  if (!base_dir.empty()) {
-    // Resolve and verify the path stays within model directory
-    auto base_canonical = std::filesystem::weakly_canonical(base_dir);
-    // If the symlink exists, it resolves to the target path;
-    // so if the symlink is outside the directory it would be caught here.
-    auto resolved = std::filesystem::weakly_canonical(base_dir / location);
-
-    // Check that resolved path starts with base directory
-    auto [base_end, resolved_it] = std::mismatch(
-        base_canonical.begin(), base_canonical.end(),
-        resolved.begin(), resolved.end());
-
-    if (base_end != base_canonical.end()) {
-      // If validation against logical base_dir fails, we check against the
-      // real (canonical) path of the model file to support symlinked models
-      // (e.g. models in Hugging Face Hub local cache).
-      if (!model_path.empty()) {
-        auto real_model_dir = std::filesystem::weakly_canonical(model_path).parent_path();
-
-        auto [real_base_end, real_resolved_it] = std::mismatch(
-            real_model_dir.begin(), real_model_dir.end(),
-            resolved.begin(), resolved.end());
-
-        if (real_base_end == real_model_dir.end()) {
-          return Status::OK();
-        }
+// Wraps std::filesystem::weakly_canonical with error_code handling.
+static Status WeaklyCanonicalPath(const std::filesystem::path& path, std::filesystem::path& result) {
+  std::error_code ec;
+  result = std::filesystem::weakly_canonical(path, ec);
+  ORT_RETURN_IF(ec, "Failed to resolve path: ", path, " - ", ec.message());
+  return Status::OK();
+}
 
-        return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
-                               "External data path: ", location, " (resolved path: ", resolved,
-                               ") escapes both model directory: ", base_dir,
-                               " and real model directory: ", real_model_dir);
-      }
+// Checks whether `child` is under `base` by comparing path components.
+static bool IsUnderDirectory(const std::filesystem::path& base, const std::filesystem::path& child) {
+  auto [base_end, child_it] = std::mismatch(base.begin(), base.end(), child.begin(), child.end());
+  return base_end == base.end();
+}
 
-      return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
-                             "External data path: ", location, " (resolved path: ", resolved,
-                             ") escapes model directory: ", base_dir);
-    }
-  } else {
-    // The basedir is empty, which occurs when 1) the session loads a model from bytes and 2) the application does not
-    // set an external file folder path via the session config option
-    // `kOrtSessionOptionsModelExternalInitializersFileFolderPath`.
+Status ValidateExternalDataPath(const std::filesystem::path& model_path,
+                                const std::filesystem::path& location) {
+  // Reject absolute paths
+  ORT_RETURN_IF(location.is_absolute(), "Absolute paths not allowed for external data location");
 
-    // We conservatively check that the normalized relative path does not contain ".." path components that would allow
-    // access to arbitrary files outside of the current working directory. Based on ONNX checker validation.
+#if defined(__wasm__)
+  if (model_path.empty()) {
+    // On WASM, filesystem utilities are not expected to always be fully supported.
+    // So, we only do a lexical check: reject ".." components that would escape the working directory.
     auto norm_location = location.lexically_normal();
-
     for (const auto& path_component : norm_location) {
       if (path_component == ORT_TSTR("..")) {
         return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "External data path: ", location,
                                " (model loaded from bytes) escapes working directory");
       }
     }
+    return Status::OK();
   }
-  return Status::OK();
+
+  // If there's a model_path on WASM, then there may be a virtual filesystem and we fallthrough
+  // to the logic below that uses std::filesystem utilities.
+#endif
+
+  // Determine the anchor directory: use model directory if provided, otherwise the current working directory.
+  std::filesystem::path anchor_dir = model_path.parent_path();
+
+  if (anchor_dir.empty()) {  // Happens if either model_path.empty() or model_path.parent_path().empty()
+    anchor_dir = ORT_TSTR(".");
+  }
+
+  // Resolve the anchor directory and the full location path to their canonical forms.
+  // This resolves symlinks so that a link pointing outside the anchor is detected.
+  std::filesystem::path anchor_canonical;
+  ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(anchor_dir, anchor_canonical));
+
+  std::filesystem::path resolved;
+  ORT_RETURN_IF_ERROR(WeaklyCanonicalPath(anchor_canonical / location, resolved));
+
+  if (IsUnderDirectory(anchor_canonical, resolved)) {
+    return Status::OK();
+  }
+
+  // Fall back to checking against the canonical model directory.
+  // This supports symlinked models (e.g., Hugging Face Hub local cache) where the canonical
+  // parent of the model file differs from the logical parent directory.
+  if (!model_path.empty()) {
+    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();
+
+    if (!real_model_dir.empty() && IsUnderDirectory(real_model_dir, resolved)) {
+      return Status::OK();
+    }
+
+    return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
+                           "External data path: ", location, " (resolved path: ", resolved,
+                           ") escapes both model directory: ", anchor_dir,
+                           " and real model directory: ", real_model_dir);
+  }
+
+  return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
+                         "External data path for model loaded from bytes escapes working directory. ",
+                         "External data path: ", location, " resolved path: ", resolved, " ",
+                         "working directory: ", anchor_dir);
 }
 
 Status GetExternalDataInfo(const ONNX_NAMESPACE::TensorProto& tensor_proto,

onnxruntime/core/framework/tensorprotoutils.h

@@ -540,18 +540,19 @@ Status TensorProtoWithExternalDataToTensorProto(
 
 /// <summary>
 /// Validates if the external data path is under the model directory.
-/// If the model is a symlink, it checks against both the logical model directory (base_dir)
+/// The model directory is derived from model_path.parent_path().
+/// If the model is a symlink, it checks against both the logical model directory
 /// and the real/canonical directory of the model.
-/// If the `base_dir` is empty, the function only ensures that `location` is not an absolute path.
+/// If model_path is empty (model loaded from bytes), the function ensures that `location` is not
+/// an absolute path, does not contain ".." components that escape the current working directory, and
+/// resolves symlinks to verify the target stays within the current working directory.
 /// </summary>
-/// <param name="base_dir">Logical model location directory</param>
+/// <param name="model_path">Path to the model file. If empty, the model was loaded from bytes.</param>
 /// <param name="location">Location string retrieved from TensorProto external data</param>
-/// <param name="model_path">Optional path to the model file, used for canonical path validation if base_dir check fails</param>
 /// <returns>The function will fail if the resolved full path is not under the logical model directory
 ///          nor the real directory of the model path</returns>
-Status ValidateExternalDataPath(const std::filesystem::path& base_dir,
-                                const std::filesystem::path& location,
-                                const std::filesystem::path& model_path = {});
+Status ValidateExternalDataPath(const std::filesystem::path& model_path,
+                                const std::filesystem::path& location);
 
 #endif  // !defined(SHARED_PROVIDER)
 

onnxruntime/core/graph/graph.cc

@@ -3742,10 +3742,6 @@ Status Graph::ConvertInitializersIntoOrtValues() {
   FindAllSubgraphs(all_subgraphs);
 
   const auto& model_path = GetModel().ModelPath();
-  PathString model_dir;
-  if (!model_path.empty()) {
-    ORT_RETURN_IF_ERROR(GetDirNameFromFilePath(model_path, model_dir));
-  }
 
   auto put_weights_maybe_in_memory_func = [&](Graph& graph) -> Status {
     // if we have any initializers that are not in memory, put them there.
@@ -3771,7 +3767,7 @@ Status Graph::ConvertInitializersIntoOrtValues() {
           std::unique_ptr<onnxruntime::ExternalDataInfo> external_data_info;
           ORT_RETURN_IF_ERROR(onnxruntime::ExternalDataInfo::Create(tensor_proto.external_data(), external_data_info));
           const auto& location = external_data_info->GetRelPath();
-          auto st = utils::ValidateExternalDataPath(model_dir, location, model_path);
+          auto st = utils::ValidateExternalDataPath(model_path, location);
           if (!st.IsOK()) {
             return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
                                    "External data path validation failed for initializer: ", tensor_proto.name(),

onnxruntime/test/framework/tensorutils_test.cc

@@ -13,6 +13,7 @@
 #include <cstdint>
 #include <limits>
 #include <fstream>
+#include <utility>
 
 #include "gtest/gtest.h"
 #include "gmock/gmock.h"
@@ -519,43 +520,73 @@ class PathValidationTest : public ::testing::Test {
     // Clean up the temporary directory.
     std::filesystem::remove_all(base_dir_);
     std::filesystem::remove_all(outside_dir_);
+
+    for (const auto& other_dir : other_dirs_) {
+      std::filesystem::remove_all(other_dir);
+    }
+  }
+
+  void AddDirToCleanUp(std::filesystem::path other_dir) {
+    other_dirs_.push_back(std::move(other_dir));
   }
 
   std::filesystem::path base_dir_;
   std::filesystem::path outside_dir_;
+  std::vector<std::filesystem::path> other_dirs_;
 };
 
 // Test cases for ValidateExternalDataPath.
 TEST_F(PathValidationTest, ValidateExternalDataPath) {
+  // Use a model path whose parent is base_dir_.
+  auto model_path = base_dir_ / "model.onnx";
+
   // Valid relative path.
-  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(base_dir_, "data.bin"));
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(model_path, "data.bin"));
 
   // Empty location.
   // Only validate it is not an absolute path.
-  ASSERT_TRUE(utils::ValidateExternalDataPath(base_dir_, "").IsOK());
+  ASSERT_TRUE(utils::ValidateExternalDataPath(model_path, "").IsOK());
 
   // Path with ".." that escapes the base directory.
-  ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "../data.bin").IsOK());
+  ASSERT_FALSE(utils::ValidateExternalDataPath(model_path, "../data.bin").IsOK());
 
   // Absolute path.
 #ifdef _WIN32
-  ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "C:\\data.bin").IsOK());
+  ASSERT_FALSE(utils::ValidateExternalDataPath(model_path, "C:\\data.bin").IsOK());
   ASSERT_FALSE(utils::ValidateExternalDataPath("", "C:\\data.bin").IsOK());
 #else
-  ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "/data.bin").IsOK());
+  ASSERT_FALSE(utils::ValidateExternalDataPath(model_path, "/data.bin").IsOK());
   ASSERT_FALSE(utils::ValidateExternalDataPath("", "/data.bin").IsOK());
 #endif  // Absolute path.
 
   // Windows vs Unix path separators.
-  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(base_dir_, "sub/data.bin"));
-  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(base_dir_, "sub\\data.bin"));
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(model_path, "sub/data.bin"));
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(model_path, "sub\\data.bin"));
 
-  // Base directory does not exist.
-  ASSERT_STATUS_OK(utils::ValidateExternalDataPath("non_existent_dir", "data.bin"));
+  // Model in a directory that does not exist.
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath("non_existent_dir/model.onnx", "data.bin"));
+
+  // Model path is a bare filename (no directory component). parent_path() returns empty,
+  // so anchor_dir falls back to "." (current directory). Path traversal should still be blocked
+  // if the current working directory is not the filesystem root directory.
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath("model.onnx", "data.bin"));
+
+  bool is_cwd_root = std::filesystem::weakly_canonical(".") == std::filesystem::weakly_canonical("..");
+  ASSERT_EQ(utils::ValidateExternalDataPath("model.onnx", "../data.bin").IsOK(), is_cwd_root);
+
+  // Model relative path checks.
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath("./model.onnx", "data.bin"));
+  ASSERT_EQ(utils::ValidateExternalDataPath("./model.onnx", "../data.bin").IsOK(), is_cwd_root);
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath("./abc/model.onnx", "data.bin"));
+#ifdef _WIN32
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(".\\model.onnx", "data.bin"));
+  ASSERT_EQ(utils::ValidateExternalDataPath(".\\model.onnx", "../data.bin").IsOK(), is_cwd_root);
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(".\\abc\\model.onnx", "data.bin"));
+#endif
 
   //
-  // Tests for an empty base directory.
-  // The base directory would be empty when 1) the session loads a model from bytes and 2) the application does not
+  // Tests for an empty model path (model loaded from bytes).
+  // The model path would be empty when 1) the session loads a model from bytes and 2) the application does not
   // set an external file folder path via the session config option
   // kOrtSessionOptionsModelExternalInitializersFileFolderPath.
   //
@@ -579,6 +610,7 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) {
 
 TEST_F(PathValidationTest, ValidateExternalDataPathWithSymlinkInside) {
   // Symbolic link that points inside the base directory.
+  auto model_path = base_dir_ / "model.onnx";
   try {
     auto target = base_dir_ / "target.bin";
     std::ofstream{target};
@@ -588,11 +620,12 @@ TEST_F(PathValidationTest, ValidateExternalDataPathWithSymlinkInside) {
     GTEST_SKIP() << "Skipping symlink tests since symlink creation is not supported in this environment. Exception: "
                  << e.what();
   }
-  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(base_dir_, "link.bin"));
+  ASSERT_STATUS_OK(utils::ValidateExternalDataPath(model_path, "link.bin"));
 }
 
 TEST_F(PathValidationTest, ValidateExternalDataPathWithSymlinkOutside) {
   // Symbolic link that points outside the base directory.
+  auto model_path = base_dir_ / "model.onnx";
   auto outside_target = outside_dir_ / "outside.bin";
   try {
     {
@@ -603,9 +636,63 @@ TEST_F(PathValidationTest, ValidateExternalDataPathWithSymlinkOutside) {
   } catch (const std::exception& e) {
     GTEST_SKIP() << "Skipping symlink tests since symlink creation is not supported in this environment. Exception: " << e.what();
   }
-  ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "outside_link.bin").IsOK());
+  ASSERT_FALSE(utils::ValidateExternalDataPath(model_path, "outside_link.bin").IsOK());
 }
 
+#if !defined(__wasm__)
+TEST_F(PathValidationTest, ValidateExternalDataPathEmptyBasedirWithSymlinkInside) {
+  // Symbolic link within the current working directory pointing to a file still within CWD.
+  std::filesystem::path cwd = std::filesystem::current_path();
+  std::filesystem::path sub_dir = cwd / "symlink_test_subdir";
+  std::filesystem::create_directories(sub_dir);
+  AddDirToCleanUp(sub_dir);
+
+  try {
+    std::filesystem::path target = sub_dir / "target_inside.bin";
+    std::filesystem::path symlink = sub_dir / "link_inside.bin";
+    std::ofstream{target};
+    std::filesystem::create_symlink(target, symlink);
+  } catch (const std::exception& e) {
+    GTEST_SKIP() << "Skipping symlink tests since symlink creation is not supported in this environment. Exception: "
+                 << e.what();
+  }
+
+  EXPECT_STATUS_OK(utils::ValidateExternalDataPath("", "./symlink_test_subdir/link_inside.bin"));
+}
+
+TEST_F(PathValidationTest, ValidateExternalDataPathEmptyBasedirWithSymlinkOutside) {
+  // Symbolic link within the current working directory pointing to a file outside CWD.
+  std::filesystem::path cwd = std::filesystem::current_path();
+  std::filesystem::path sub_dir = cwd / "symlink_test_subdir2";
+  std::filesystem::create_directories(sub_dir);
+  AddDirToCleanUp(sub_dir);
+
+  // Check if we can actually make a file outside of the current working directory (i.e., in a temp dir).
+  // This is only possible if the current working directory is NOT the same as the temp directory.
+  // Otherwise, we need to skip this test. This happens in Android CI.
+  auto [cwd_end, outside_end] = std::mismatch(cwd.begin(), cwd.end(), outside_dir_.begin(), outside_dir_.end());
+  if (cwd_end == cwd.end()) {
+    GTEST_SKIP() << "Skipping test that needs to create a symlink outside of the cwd because the cwd is the same as "
+                 << "the temp dir. cwd: " << cwd << " outside_dir_: " << outside_dir_;
+  }
+
+  try {
+    std::filesystem::path outside_target = outside_dir_ / "outside_for_empty_basedir.bin";
+    std::filesystem::path symlink = sub_dir / "outside_link.bin";
+    std::ofstream{outside_target};
+    std::filesystem::create_symlink(outside_target, symlink);
+  } catch (const std::exception& e) {
+    GTEST_SKIP() << "Skipping symlink tests since symlink creation is not supported in this environment. Exception: "
+                 << e.what();
+  }
+
+  Status status = utils::ValidateExternalDataPath("", "./symlink_test_subdir2/outside_link.bin");
+  ASSERT_FALSE(status.IsOK()) << "Expected validation to fail. cwd: " << cwd << " sub_dir: " << sub_dir
+                              << " outside_dir: " << outside_dir_;
+  EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("escapes working directory"));
+}
+#endif  // !defined(__wasm__)
+
 // Tests for ValidateEmbeddedTensorProtoDataSizeAndShape and embedded initializer size limits
 
 TEST(TensorProtoDataSizeShapeValidationTest, ValidTensorProtoWithRawData) {