Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions onnxruntime/core/framework/tensorprotoutils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,21 @@ 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 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();

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();
}
Expand Down
26 changes: 23 additions & 3 deletions onnxruntime/test/framework/tensorutils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -555,6 +552,29 @@ 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 ".." 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());

// 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) {
Expand Down
87 changes: 87 additions & 0 deletions onnxruntime/test/shared_lib/test_inference.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4918,6 +4918,93 @@ TEST(CApiTest, ModelWithExternalDataOutsideModelDirectoryShouldFailToLoad) {
<< "Exception message should indicate external data or security issue. Got: " << exception_message;
}

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
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<size_t>(model_file_stream.tellg());
model_file_stream.seekg(0, std::ios::beg);
std::vector<char> 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, 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.
// 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<size_t>(model_file_stream.tellg());
model_file_stream.seekg(0, std::ios::beg);
std::vector<char> 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

Expand Down
Loading