From 7a04a411f70c80802c07921ed6be66912d0392e1 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Mon, 23 Feb 2026 17:11:28 -0800 Subject: [PATCH 1/3] Fix validation for external data paths for models loaded from bytes --- .../core/framework/tensorprotoutils.cc | 8 ++ .../test/framework/tensorutils_test.cc | 6 ++ onnxruntime/test/shared_lib/test_inference.cc | 87 +++++++++++++++++++ 3 files changed, 101 insertions(+) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 961012536126b..f081b869a7058 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -371,6 +371,14 @@ Status ValidateExternalDataPath(const std::filesystem::path& base_dir, "External data path: ", location, " (resolved path: ", resolved, ") escapes model directory: ", base_dir); } + } else { + // The basedir is empty, which occurs when a model is loaded from memory. + // We check that the path does not contain ".." that would otherwise allow access to arbitrary files. + auto norm_location = location.lexically_normal(); + if (norm_location.native().find(ORT_TSTR(".."), 0) != std::basic_string::npos) { + return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "External data path: ", location, + " (model loaded from bytes) escapes working directory"); + } } return Status::OK(); } diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 0d7b583faf27b..7bf6096c4274d 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -540,6 +540,12 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) { // Path with ".." that escapes the base directory. ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "../data.bin").IsOK()); + // Path with ".." that escapes the current working directory. Base directory is empty when: + // - Session loads a model from bytes + // - Application does not set an external file folder path via the session config option + // kOrtSessionOptionsModelExternalInitializersFileFolderPath. + ASSERT_FALSE(utils::ValidateExternalDataPath("", "../data.bin").IsOK()); + // Absolute path. #ifdef _WIN32 ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "C:\\data.bin").IsOK()); diff --git a/onnxruntime/test/shared_lib/test_inference.cc b/onnxruntime/test/shared_lib/test_inference.cc index 4e991716dd108..5519d22bc9af4 100644 --- a/onnxruntime/test/shared_lib/test_inference.cc +++ b/onnxruntime/test/shared_lib/test_inference.cc @@ -4918,6 +4918,93 @@ TEST(CApiTest, ModelWithExternalDataOutsideModelDirectoryShouldFailToLoad) { << "Exception message should indicate external data or security issue. Got: " << exception_message; } +TEST(CApiTest, InMemoryModelWithExternalDataOutsideWorkingDirectoryShouldFailToLoad) { + // Attempt to create an ORT session with the malicious model (loaded from bytes). + // This should fail due to the use of an external file path that is not under current working directory. + // i.e. ../../../../etc/passwd + constexpr const ORTCHAR_T* model_path = TSTR("testdata/test_arbitrary_external_file.onnx"); + + Ort::Env env(ORT_LOGGING_LEVEL_WARNING, "test"); + Ort::SessionOptions session_options; + + // Load model contents into array + std::ifstream model_file_stream(model_path, std::ios::in | std::ios::binary); + ASSERT_TRUE(model_file_stream.good()); + model_file_stream.seekg(0, std::ios::end); + const auto file_contents_size = onnxruntime::narrow(model_file_stream.tellg()); + model_file_stream.seekg(0, std::ios::beg); + std::vector file_contents(file_contents_size, 0); + model_file_stream.read(&file_contents[0], file_contents_size); + model_file_stream.close(); + + bool exception_thrown = false; + std::string exception_message; + + try { + // This should throw an exception due to malicious external data + Ort::Session session(env, file_contents.data(), file_contents_size, session_options); + } catch (const Ort::Exception& e) { + exception_thrown = true; + exception_message = e.what(); + } catch (const std::exception& e) { + exception_thrown = true; + exception_message = e.what(); + } + + // Verify that loading the model failed + EXPECT_TRUE(exception_thrown) << "Expected model loading to fail due to malicious external data path"; + + // Verify that the exception message indicates security or external data issues + EXPECT_TRUE(exception_message.find("External data path") != std::string::npos && + exception_message.find("escapes working directory") != std::string::npos) + << "Exception message should indicate external data or security issue. Got: " << exception_message; +} + +TEST(CApiTest, InMemoryModelWithExternalDataOutsideModelDirectoryShouldFailToLoad) { + // Attempt to create an ORT session with the malicious model (loaded from bytes). + // A valid external file folder path is explicitly set via session options. + // However, this should still fail due to the use of an external file path that escapes the set directory. + // i.e. ../../../../etc/passwd + constexpr const ORTCHAR_T* model_path = TSTR("testdata/test_arbitrary_external_file.onnx"); + + Ort::Env env(ORT_LOGGING_LEVEL_WARNING, "test"); + Ort::SessionOptions session_options; + session_options.AddConfigEntry(kOrtSessionOptionsModelExternalInitializersFileFolderPath, "testdata"); + + // Load model contents into array + std::ifstream model_file_stream(model_path, std::ios::in | std::ios::binary); + ASSERT_TRUE(model_file_stream.good()); + model_file_stream.seekg(0, std::ios::end); + const auto file_contents_size = onnxruntime::narrow(model_file_stream.tellg()); + model_file_stream.seekg(0, std::ios::beg); + std::vector file_contents(file_contents_size, 0); + model_file_stream.read(&file_contents[0], file_contents_size); + model_file_stream.close(); + + bool exception_thrown = false; + std::string exception_message; + + try { + // This should throw an exception due to malicious external data + Ort::Session session(env, file_contents.data(), file_contents_size, session_options); + } catch (const Ort::Exception& e) { + exception_thrown = true; + exception_message = e.what(); + } catch (const std::exception& e) { + exception_thrown = true; + exception_message = e.what(); + } + + // Verify that loading the model failed + EXPECT_TRUE(exception_thrown) << "Expected model loading to fail due to malicious external data path"; + + // Verify that the exception message indicates security or external data issues + EXPECT_TRUE(exception_message.find("External data path") != std::string::npos && + exception_message.find("escapes both model directory") != std::string::npos && + exception_message.find("and real model directory") != std::string::npos) + << "Exception message should indicate external data or security issue. Got: " << exception_message; +} + #ifdef ORT_ENABLE_STREAM #if USE_CUDA From d4673efb761aefec88362ae810f10a16cecb0926 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Mon, 23 Feb 2026 21:59:00 -0800 Subject: [PATCH 2/3] Address ghc review comments --- .../core/framework/tensorprotoutils.cc | 9 ++++-- .../test/framework/tensorutils_test.cc | 29 +++++++++++++------ onnxruntime/test/shared_lib/test_inference.cc | 4 +-- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index f081b869a7058..7c03bbb74736e 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -372,9 +372,14 @@ Status ValidateExternalDataPath(const std::filesystem::path& base_dir, ") escapes model directory: ", base_dir); } } else { - // The basedir is empty, which occurs when a model is loaded from memory. - // We check that the path does not contain ".." that would otherwise allow access to arbitrary files. + // 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`. + + // 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. auto norm_location = location.lexically_normal(); + if (norm_location.native().find(ORT_TSTR(".."), 0) != std::basic_string::npos) { return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "External data path: ", location, " (model loaded from bytes) escapes working directory"); diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 7bf6096c4274d..36aeb92ab9cfa 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -530,9 +530,6 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) { // Valid relative path. ASSERT_STATUS_OK(utils::ValidateExternalDataPath(base_dir_, "data.bin")); - // Empty base directory. - ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "data.bin")); - // Empty location. // Only validate it is not an absolute path. ASSERT_TRUE(utils::ValidateExternalDataPath(base_dir_, "").IsOK()); @@ -540,12 +537,6 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) { // Path with ".." that escapes the base directory. ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "../data.bin").IsOK()); - // Path with ".." that escapes the current working directory. Base directory is empty when: - // - Session loads a model from bytes - // - Application does not set an external file folder path via the session config option - // kOrtSessionOptionsModelExternalInitializersFileFolderPath. - ASSERT_FALSE(utils::ValidateExternalDataPath("", "../data.bin").IsOK()); - // Absolute path. #ifdef _WIN32 ASSERT_FALSE(utils::ValidateExternalDataPath(base_dir_, "C:\\data.bin").IsOK()); @@ -561,6 +552,26 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) { // Base directory does not exist. ASSERT_STATUS_OK(utils::ValidateExternalDataPath("non_existent_dir", "data.bin")); + + // + // 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 + // set an external file folder path via the session config option + // kOrtSessionOptionsModelExternalInitializersFileFolderPath. + // + + // A simple filename is ok (would not escape current working directory). + ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "data.bin")); + ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "./data.bin")); + + // A path that would escape the current working directory is invalid. + ASSERT_FALSE(utils::ValidateExternalDataPath("", "../data.bin").IsOK()); + + // A path that uses ".." but would not escape the current working directory should be fine. + ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "a/../data.bin")); + + // A path with multiple internal ".." that would escape current working direction should fail. + ASSERT_FALSE(utils::ValidateExternalDataPath("", "a/../../data.bin").IsOK()); } TEST_F(PathValidationTest, ValidateExternalDataPathWithSymlinkInside) { diff --git a/onnxruntime/test/shared_lib/test_inference.cc b/onnxruntime/test/shared_lib/test_inference.cc index 5519d22bc9af4..e472cbcee12d6 100644 --- a/onnxruntime/test/shared_lib/test_inference.cc +++ b/onnxruntime/test/shared_lib/test_inference.cc @@ -4918,7 +4918,7 @@ TEST(CApiTest, ModelWithExternalDataOutsideModelDirectoryShouldFailToLoad) { << "Exception message should indicate external data or security issue. Got: " << exception_message; } -TEST(CApiTest, InMemoryModelWithExternalDataOutsideWorkingDirectoryShouldFailToLoad) { +TEST(CApiTest, InMemoryModel_ExternalDataOutsideWorkingDirectory_FailToLoad) { // Attempt to create an ORT session with the malicious model (loaded from bytes). // This should fail due to the use of an external file path that is not under current working directory. // i.e. ../../../../etc/passwd @@ -4960,7 +4960,7 @@ TEST(CApiTest, InMemoryModelWithExternalDataOutsideWorkingDirectoryShouldFailToL << "Exception message should indicate external data or security issue. Got: " << exception_message; } -TEST(CApiTest, InMemoryModelWithExternalDataOutsideModelDirectoryShouldFailToLoad) { +TEST(CApiTest, InMemoryModel_SessionConfigExternalFileFolder_ExternalDataOutsideModelDirectory_FailToLoad) { // Attempt to create an ORT session with the malicious model (loaded from bytes). // A valid external file folder path is explicitly set via session options. // However, this should still fail due to the use of an external file path that escapes the set directory. From 648312140bbd5c0748d61e43a8f800800d36c1bd Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Tue, 24 Feb 2026 09:37:01 -0800 Subject: [PATCH 3/3] Address comment about .. as part of the filename --- onnxruntime/core/framework/tensorprotoutils.cc | 8 +++++--- onnxruntime/test/framework/tensorutils_test.cc | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 7c03bbb74736e..e4c7830ffbb55 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -380,9 +380,11 @@ Status ValidateExternalDataPath(const std::filesystem::path& base_dir, // access to arbitrary files outside of the current working directory. Based on ONNX checker validation. auto norm_location = location.lexically_normal(); - if (norm_location.native().find(ORT_TSTR(".."), 0) != std::basic_string::npos) { - return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "External data path: ", location, - " (model loaded from bytes) escapes working directory"); + 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(); diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 36aeb92ab9cfa..c9b61a7a39632 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -564,6 +564,9 @@ TEST_F(PathValidationTest, ValidateExternalDataPath) { ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "data.bin")); ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "./data.bin")); + // A ".." that is not a path component (part of the filename) is ok + ASSERT_STATUS_OK(utils::ValidateExternalDataPath("", "data..bin")); + // A path that would escape the current working directory is invalid. ASSERT_FALSE(utils::ValidateExternalDataPath("", "../data.bin").IsOK());