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
7 changes: 6 additions & 1 deletion onnxruntime/core/framework/tensor_external_data_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Status ExternalDataInfo::Create(const RepeatedPtrField<StringStringEntryProto>&
std::unique_ptr<ExternalDataInfo>& external_data_info_result) {
auto external_data_info = std::make_unique<ExternalDataInfo>();
PrepackedInfos prepacked_infos;
bool has_location = false;

const int input_size = input.size();

Expand All @@ -39,7 +40,11 @@ Status ExternalDataInfo::Create(const RepeatedPtrField<StringStringEntryProto>&
if (!stringmap.has_value())
return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "model format error! Need a value for the external data info");

if (stringmap.key() == "location" && !stringmap.value().empty()) {
if (stringmap.key() == "location") {
ORT_RETURN_IF(has_location,
"model format error! TensorProto external data has duplicate 'location' entries");
has_location = true;
ORT_RETURN_IF(stringmap.value().empty(), "model format error! External data location cannot be empty");
external_data_info->rel_path_ = ToWideString(stringmap.value());
} else if (stringmap.key() == "offset" && !stringmap.value().empty()) {
ORT_RETURN_IF_ERROR(ParseStringWithClassicLocale(stringmap.value(), external_data_info->offset_));
Expand Down
42 changes: 26 additions & 16 deletions onnxruntime/core/framework/tensorprotoutils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,13 @@ namespace utils {

bool HasExternalDataInMemory(const ONNX_NAMESPACE::TensorProto& ten_proto) {
if (HasExternalData(ten_proto)) {
// Retrieve the external data info
for (const auto& entry : ten_proto.external_data()) {
if (entry.key() == "location") {
PathString location = ToWideString(entry.value());
return ((location == kTensorProtoLittleEndianMemoryAddressTag) || (location == kTensorProtoNativeEndianMemoryAddressTag));
const PathString location = ToWideString(entry.value());
if (location == kTensorProtoLittleEndianMemoryAddressTag ||
location == kTensorProtoNativeEndianMemoryAddressTag) {
return true;
}
}
}
}
Expand Down Expand Up @@ -407,11 +409,12 @@ static bool HasPathComponentPrefix(const std::filesystem::path& prefix, const st
///
/// 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
/// 2. Reject internal in-memory reference tags
/// 3. Reject absolute paths (including Unix-style '/...' on Windows)
/// 4. Skip remaining checks on WASM if no filesystem is available
/// 5. Resolve `model_dir / external_data_path` to a canonical path (resolving symlinks for existing segments)
/// 6. Verify the canonical path is a prefix-child of the canonical model_dir (containment check)
/// 7. 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.
Expand All @@ -420,13 +423,18 @@ Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir,
// Step 1: Reject empty external data paths.
ORT_RETURN_IF(external_data_path.empty(), "Empty external data path not allowed");

// Step 2: Reject absolute paths.
// Step 2: Reject internal in-memory reference tags.
ORT_RETURN_IF(external_data_path.native() == kTensorProtoLittleEndianMemoryAddressTag ||
external_data_path.native() == kTensorProtoNativeEndianMemoryAddressTag,
"In-memory external data reference tag is not a valid file path");

// Step 3: 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
// Step 4 (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);
Expand All @@ -435,7 +443,7 @@ Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir,
}
#endif

// Step 4: Resolve both the model directory and the combined path to canonical forms.
// Step 5: 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;
Expand All @@ -445,8 +453,8 @@ Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir,
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));

// 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.
// Step 6: Containment check — verify the resolved external data path starts with the model directory.
// Step 7: 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));
Expand All @@ -466,7 +474,7 @@ Status ValidateExternalDataPathFromDir(const std::filesystem::path& model_dir,
/// 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
/// 2. If it fails due to an empty/absolute path or an in-memory reference tag, 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:
Expand Down Expand Up @@ -494,9 +502,11 @@ Status ValidateExternalDataPath(const std::filesystem::path& model_path,
}

// --- Guard: Don't retry for input-validation errors ---
// Empty and absolute paths are always invalid regardless of model directory or symlinks.
// Empty paths, absolute paths, and in-memory reference tags are always invalid regardless of model directory.
// Return the error directly without misleading "escapes directory" context.
if (external_data_path.empty() || !external_data_path.root_path().empty()) {
if (external_data_path.empty() || !external_data_path.root_path().empty() ||
external_data_path.native() == kTensorProtoLittleEndianMemoryAddressTag ||
external_data_path.native() == kTensorProtoNativeEndianMemoryAddressTag) {
return status;
}

Expand Down
56 changes: 56 additions & 0 deletions onnxruntime/test/framework/tensorutils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,50 @@ TEST(TensorProtoUtilsTest, ParseExternalDataInfoOffsetAndLength) {
// TODO should ExternalDataInfo::Create() also reject negative offset values?
}

TEST(TensorProtoUtilsTest, ExternalDataInfoRejectsDuplicateLocations) {
const std::string memory_tag = ToUTF8String(utils::kTensorProtoNativeEndianMemoryAddressTag);
const std::array<std::pair<std::string, std::string>, 3> locations{{
{"data.bin", "data.bin"},
{"data.bin", memory_tag},
{memory_tag, "data.bin"},
}};

for (const auto& [first_location, second_location] : locations) {
ONNX_NAMESPACE::TensorProto tensor_proto;
tensor_proto.set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);
auto* first = tensor_proto.add_external_data();
first->set_key("location");
first->set_value(first_location);
auto* second = tensor_proto.add_external_data();
second->set_key("location");
second->set_value(second_location);

std::unique_ptr<ExternalDataInfo> external_data_info;
const Status status = ExternalDataInfo::Create(tensor_proto.external_data(), external_data_info);
ASSERT_STATUS_NOT_OK_AND_HAS_SUBSTR(status, "duplicate 'location'");
}
}

TEST(TensorProtoUtilsTest, HasExternalDataInMemoryChecksAllLocations) {
auto make_tensor = [](const std::string& first_location, const std::string& second_location) {
ONNX_NAMESPACE::TensorProto tensor_proto;
tensor_proto.set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
tensor_proto.set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);
auto* first = tensor_proto.add_external_data();
first->set_key("location");
first->set_value(first_location);
auto* second = tensor_proto.add_external_data();
second->set_key("location");
second->set_value(second_location);
return tensor_proto;
};

const std::string memory_tag = ToUTF8String(utils::kTensorProtoNativeEndianMemoryAddressTag);
EXPECT_TRUE(utils::HasExternalDataInMemory(make_tensor("data.bin", memory_tag)));
EXPECT_TRUE(utils::HasExternalDataInMemory(make_tensor(memory_tag, "data.bin")));
EXPECT_TRUE(utils::HasExternalDataInMemory(make_tensor(memory_tag, "")));
}

// Test ExternalData functionality
TEST(TensorProtoUtilsTest, SetExternalDataInformation) {
ONNX_NAMESPACE::TensorProto tensor_proto;
Expand Down Expand Up @@ -725,6 +769,18 @@ class PathValidationTest : public ::testing::Test {
std::vector<std::filesystem::path> other_files_;
};

TEST_F(PathValidationTest, ValidateExternalDataPathRejectsMemoryTags) {
for (const auto* memory_tag : {utils::kTensorProtoLittleEndianMemoryAddressTag,
utils::kTensorProtoNativeEndianMemoryAddressTag}) {
const Status status = utils::ValidateExternalDataPathFromDir(base_dir_, std::filesystem::path{memory_tag});
ASSERT_STATUS_NOT_OK_AND_HAS_SUBSTR(status, "In-memory external data reference tag");

const Status wrapper_status =
utils::ValidateExternalDataPath(base_dir_ / "model.onnx", std::filesystem::path{memory_tag});
ASSERT_STATUS_NOT_OK_AND_HAS_SUBSTR(wrapper_status, "In-memory external data reference tag");
}
}

// Test cases for ValidateExternalDataPath.
TEST_F(PathValidationTest, ValidateExternalDataPath) {
std::filesystem::path model_path = base_dir_ / "model.onnx";
Expand Down
Loading