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
83 changes: 62 additions & 21 deletions onnxruntime/core/framework/tensorprotoutils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1565,13 +1565,35 @@ static Status GetFileContent(const Env& env, const std::filesystem::path& file_p
}
#endif

// Backstop validation for callers that load external data outside Graph::Resolve (e.g. training
// checkpoints, custom-op initializers). Passes through ORT's in-memory address markers — those are
// validated at higher layers (Graph::ConvertInitializersIntoOrtValues for dense; markers on sparse
// sub-tensors are rejected outright in SparseTensorProtoToDenseTensorProto). For declared file paths,
// defers to ValidateExternalDataPath, which rejects absolute paths and paths that escape the model
// directory. Callers must have already verified the tensor has external data.
static Status ValidateExternalFilePathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto,
const std::filesystem::path& model_path) {
if (HasExternalDataInMemory(tensor_proto)) {
return Status::OK();
}

std::unique_ptr<ExternalDataInfo> external_data_info;
ORT_RETURN_IF_ERROR(ExternalDataInfo::Create(tensor_proto.external_data(), external_data_info));
return utils::ValidateExternalDataPath(model_path, external_data_info->GetRelPath());
}

Status GetExtDataFromTensorProto(const Env& env,
const std::filesystem::path& model_path,
const ONNX_NAMESPACE::TensorProto& tensor_proto,
OrtValue& ort_value, PrepackedWeightsForGraph* prepacked_info) {
ORT_ENFORCE(HasExternalData(tensor_proto), "TensorProto for: ",
tensor_proto.name(), "Expected to have external data");

// Defense-in-depth: reject absolute or directory-escaping external data paths even when this
// function is reached outside Graph::Resolve (e.g. training checkpoint load, custom-op init).
// In-memory address markers are passed through; their validity is enforced upstream.
ORT_RETURN_IF_ERROR(ValidateExternalFilePathForTensor(tensor_proto, model_path));

std::basic_string<ORTCHAR_T> tensor_proto_dir;
if (!model_path.empty()) {
ORT_RETURN_IF_ERROR(GetDirNameFromFilePath(model_path, tensor_proto_dir));
Expand Down Expand Up @@ -1735,6 +1757,9 @@ Status LoadExtDataToTensorFromTensorProto(const Env& env, const std::filesystem:
const IExternalDataLoader& ext_data_loader,
Tensor& tensor) {
ORT_ENFORCE(HasExternalData(tensor_proto));
// Defense-in-depth path validation for callers reaching this function outside Graph::Resolve.
// In-memory markers are passed through; rejected explicitly below as unsupported for this path.
ORT_RETURN_IF_ERROR(ValidateExternalFilePathForTensor(tensor_proto, model_path));
std::basic_string<ORTCHAR_T> tensor_proto_dir;
if (!model_path.empty()) {
ORT_RETURN_IF_ERROR(GetDirNameFromFilePath(model_path, tensor_proto_dir));
Expand Down Expand Up @@ -2098,30 +2123,29 @@ void MakeCpuTensorCopy(const Tensor& src_tensor, Tensor& dst_tensor) {

#if !defined(DISABLE_SPARSE_TENSORS)

// Validates that a TensorProto's external data path does not escape the model directory.
// Also validates that the file exists when filesystem access is available (skipped on WASM without a virtual FS).
// Returns Status::OK() (no-op) for tensors that do not use file-based external data.
static Status ValidateExternalDataPathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto,
const std::filesystem::path& model_path) {
// Gates on data_location == EXTERNAL directly instead of using HasExternalData()/HasExternalDataInFile(),
// which also require data_type != UNDEFINED. That check is appropriate for data processing (can't unpack
// without a type), but too narrow for security validation: we must validate any declared external path
// regardless of data_type.
if (tensor_proto.data_location() != ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL) {
// Validates the external data declaration on a sub-tensor of a SparseTensorProto (values or
// indices). Validates that any file path stays within the model directory.
//
// Gates on data_location == EXTERNAL (rather than HasExternalData()) so that path validation
// runs even when data_type is UNDEFINED. A malicious model could set data_location=EXTERNAL with
// data_type=UNDEFINED and an evil file path; downstream loading would also reject it, but we
// validate here for defense-in-depth.
//
// In-memory address markers must never appear on sparse sub-tensors. The trusted .ort loader
// materializes sparse sub-tensors as inline raw_data (see LoadSparseInitializerOrtFormat); the
// untrusted .onnx protobuf path rejects markers at the Graph constructor; and
// SparseTensorProtoToDenseTensorProto re-asserts the invariant before this function is reached.
// The HasExternalDataInMemory early-return below is a paranoid backstop.
static Status ValidateSparseSubTensorExternalDataPath(const ONNX_NAMESPACE::TensorProto& tensor_proto,
const std::filesystem::path& model_path) {
Comment thread
yuslepukhin marked this conversation as resolved.
if (tensor_proto.data_location() != ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL ||
HasExternalDataInMemory(tensor_proto)) {
return Status::OK();
}

std::unique_ptr<ExternalDataInfo> external_data_info;
ORT_RETURN_IF_ERROR(ExternalDataInfo::Create(tensor_proto.external_data(), external_data_info));
const auto& rel_path = external_data_info->GetRelPath();

// In-memory external data uses special marker locations — skip file path validation for those.
if (rel_path == kTensorProtoLittleEndianMemoryAddressTag ||
rel_path == kTensorProtoNativeEndianMemoryAddressTag) {
return Status::OK();
}

return utils::ValidateExternalDataPath(model_path, rel_path);
return utils::ValidateExternalDataPath(model_path, external_data_info->GetRelPath());
}

static Status CopySparseData(const std::string& name,
Expand Down Expand Up @@ -2303,6 +2327,23 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT
const auto& sparse_values = sparse.values();
const auto& name = sparse_values.name();

// In-memory address markers (pointing into mmap'd / heap buffers) are forbidden on sparse
// sub-tensors. The trusted .ort loader is required to materialize sparse sub-tensors as inline
// raw_data (see LoadSparseInitializerOrtFormat) so they never carry markers. Untrusted .onnx
// protobuf input is rejected at the Graph constructor before reaching this function; this is
// the function-level backstop. A marker here would otherwise trigger an arbitrary memory read
// in UnpackInitializerData.
if (HasExternalDataInMemory(sparse_values)) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH,
"Sparse tensor: ", name,
" values use an in-memory address marker which is not permitted on sparse sub-tensors.");
}
if (HasExternalDataInMemory(sparse.indices())) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH,
"Sparse tensor: ", name,
" indices use an in-memory address marker which is not permitted on sparse sub-tensors.");
}

const auto values_rank = sparse_values.dims_size();
if (values_rank != 1) {
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_GRAPH,
Expand Down Expand Up @@ -2371,8 +2412,8 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT
// Validate external data paths before any early returns or allocations.
// This ensures malicious paths are rejected even for zero-element tensors,
// and prevents large allocations before an invalid path is caught.
ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(sparse_values, model_path));
ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(indices, model_path));
ORT_RETURN_IF_ERROR(ValidateSparseSubTensorExternalDataPath(sparse_values, model_path));
ORT_RETURN_IF_ERROR(ValidateSparseSubTensorExternalDataPath(indices, model_path));

if (dense_elements == 0) {
// if there are no elements in the dense tensor, we can return early with an empty tensor proto
Expand Down
33 changes: 33 additions & 0 deletions onnxruntime/core/graph/graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,21 @@ Graph::Graph(const Model& owning_model,
continue;
}

#if !defined(DISABLE_SPARSE_TENSORS)
// Reject ORT in-memory address markers on a sparse-tensor Constant attribute before the
// sparse-to-dense conversion runs — those markers are an in-process ORT sentinel and must
// never appear in a deserialized protobuf. See note on the dense initializer loop below.
if (node.attribute_size() > 0 &&
node.attribute(0).type() == AttributeProto_AttributeType_SPARSE_TENSOR) {
const auto& s = node.attribute(0).sparse_tensor();
ORT_ENFORCE(!utils::HasExternalDataInMemory(s.values()) &&
!utils::HasExternalDataInMemory(s.indices()),
"Constant node '", node.name(),
"' sparse-tensor attribute references an ORT in-memory address marker, "
"which is not allowed in a model protobuf.");
}
#endif

const gsl::not_null<TensorProto*> tensor{graph_proto_->add_initializer()};
ORT_THROW_IF_ERROR(utils::ConstantNodeProtoToTensorProto(node, model_path, *tensor));

Expand Down Expand Up @@ -1304,6 +1319,16 @@ Graph::Graph(const Model& owning_model,
if (graph_proto_->sparse_initializer_size() > 0) {
for (const auto& sparse_tensor : graph_proto_->sparse_initializer()) {
ORT_ENFORCE(utils::HasName(sparse_tensor), "Sparse initializer must have a name. This model is invalid");
// Reject ORT's in-memory address markers on sparse sub-tensors arriving via the protobuf
// path. Such markers are an internal ORT optimization set by trusted loaders (e.g. ORT-format
// flatbuffer load) and must never appear in a SparseTensorProto deserialized from an .onnx
// protobuf; if they do, the model is crafted and would cause ORT to dereference an
// attacker-supplied pointer during sparse-to-dense conversion.
for (const auto* sub : {&sparse_tensor.values(), &sparse_tensor.indices()}) {
ORT_ENFORCE(!utils::HasExternalDataInMemory(*sub),
"Sparse initializer '", sparse_tensor.values().name(),
"' references an ORT in-memory address marker, which is not allowed in a model protobuf.");
}
const gsl::not_null<TensorProto*> tensor{graph_proto_->add_initializer()};
auto status = utils::SparseTensorProtoToDenseTensorProto(sparse_tensor, model_path, *tensor);
ORT_ENFORCE(status.IsOK(), status.ToString());
Expand Down Expand Up @@ -1345,6 +1370,14 @@ Graph::Graph(const Model& owning_model,

// Copy initial tensors to a map.
for (auto& tensor : graph_proto_->initializer()) {
// ORT in-memory address markers are an in-process sentinel: they can only be planted by ORT
// itself (e.g. when constructing a TensorProto that aliases an mmap'd .ort buffer or an OrtValue).
// They must never appear in a TensorProto deserialized from an .onnx protobuf — if they do, the
// model is crafted and would cause ORT to dereference an attacker-supplied pointer when
// resolving the initializer.
ORT_ENFORCE(!utils::HasExternalDataInMemory(tensor),
"Initializer '", tensor.name(),
"' references an ORT in-memory address marker, which is not allowed in a model protobuf.");
auto p = name_to_initial_tensor_.emplace(tensor.name(), &tensor);
if (!p.second) {
LOGS(logger_, WARNING) << "Duplicate initializer (dense, sparse or ConstantNode): '" << tensor.name()
Expand Down
14 changes: 12 additions & 2 deletions onnxruntime/core/graph/graph_flatbuffers_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,17 +418,27 @@ Status LoadSparseInitializerOrtFormat(const fbs::SparseTensor& fbs_sparse_tensor
SparseTensorProto& initializer,
const OrtFormatLoadOptions& load_options) {
SparseTensorProto loaded_initializer;

// Sparse sub-tensors must never carry the in-memory address marker. The marker would point into
// the mmap'd flatbuffer buffer; allowing it here would force every downstream consumer of the
// sparse->dense conversion to validate the marker, and would conflate the trust boundary
// (sparse markers from untrusted .onnx input are an arbitrary-memory-read vector). Force the
// inner loader to materialize a normal inline raw_data copy regardless of size; the cost is
// small because sparse->dense conversion immediately copies the bytes again.
OrtFormatLoadOptions sub_tensor_options = load_options;
sub_tensor_options.can_use_flatbuffer_for_initializers = false;

auto fbs_values_tensor = fbs_sparse_tensor.values();
ORT_RETURN_IF(nullptr == fbs_values_tensor, "Missing values for sparse initializer. Invalid ORT format model.");
auto* values_tensor = loaded_initializer.mutable_values();
ORT_RETURN_IF_ERROR(LoadInitializerOrtFormat(*fbs_values_tensor, *values_tensor, load_options));
ORT_RETURN_IF_ERROR(LoadInitializerOrtFormat(*fbs_values_tensor, *values_tensor, sub_tensor_options));
ORT_RETURN_IF(values_tensor->name().empty(), "Missing name for SparseTensor initializer. Invalid ORT format model.");

auto fbs_indicies_tensor = fbs_sparse_tensor.indices();
ORT_RETURN_IF(nullptr == fbs_indicies_tensor, "Missing indicies for sparse initializer: ", "'", values_tensor->name(), "'",
"Invalid ORT format model.");
auto* indicies_tensor = loaded_initializer.mutable_indices();
ORT_RETURN_IF_ERROR(LoadInitializerOrtFormat(*fbs_indicies_tensor, *indicies_tensor, load_options));
ORT_RETURN_IF_ERROR(LoadInitializerOrtFormat(*fbs_indicies_tensor, *indicies_tensor, sub_tensor_options));

auto fbs_dims = fbs_sparse_tensor.dims();
ORT_RETURN_IF(nullptr == fbs_dims, "Missing dims for sparse initializer: ", "'", values_tensor->name(), "'",
Expand Down
129 changes: 129 additions & 0 deletions onnxruntime/test/framework/tensorutils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1151,8 +1151,137 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroNNZ)
EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes"));
}

// Defense-in-depth: SparseTensorProtoToDenseTensorProto must reject ORT's in-memory address
// marker on sparse sub-tensors unconditionally. The trusted .ort loader is required to
// materialize sparse sub-tensors as inline raw_data so they never carry markers. Without this
// self-check, a caller that bypasses the Graph-ctor chokepoint would dereference an
// attacker-controlled address.
TEST(SparseTensorProtoToDenseTensorProtoMarkerTest, RejectsInMemoryMarkerOnValuesByDefault) {
ONNX_NAMESPACE::SparseTensorProto sparse;
sparse.add_dims(4);

auto* values = sparse.mutable_values();
values->set_name("sparse_marker_values");
values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
values->add_dims(2);
values->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);
auto* loc = values->add_external_data();
loc->set_key("location");
loc->set_value(ToUTF8String(onnxruntime::utils::kTensorProtoLittleEndianMemoryAddressTag));
auto* off = values->add_external_data();
off->set_key("offset");
off->set_value("0");
auto* len = values->add_external_data();
len->set_key("length");
len->set_value(std::to_string(2 * sizeof(float)));

auto* indices = sparse.mutable_indices();
indices->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_INT64);
indices->add_dims(2);
indices->add_int64_data(0);
indices->add_int64_data(1);

ONNX_NAMESPACE::TensorProto dense;
Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, std::filesystem::path{}, dense);
ASSERT_FALSE(status.IsOK());
EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("in-memory address marker"));
}

TEST(SparseTensorProtoToDenseTensorProtoMarkerTest, RejectsInMemoryMarkerOnIndicesByDefault) {
ONNX_NAMESPACE::SparseTensorProto sparse;
sparse.add_dims(4);

auto* values = sparse.mutable_values();
values->set_name("sparse_marker_indices");
values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
values->add_dims(2);
values->add_float_data(1.0f);
values->add_float_data(2.0f);

auto* indices = sparse.mutable_indices();
indices->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_INT64);
indices->add_dims(2);
indices->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);
auto* loc = indices->add_external_data();
loc->set_key("location");
loc->set_value(ToUTF8String(onnxruntime::utils::kTensorProtoLittleEndianMemoryAddressTag));
auto* off = indices->add_external_data();
off->set_key("offset");
off->set_value("0");
auto* len = indices->add_external_data();
len->set_key("length");
len->set_value(std::to_string(2 * sizeof(int64_t)));

ONNX_NAMESPACE::TensorProto dense;
Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, std::filesystem::path{}, dense);
ASSERT_FALSE(status.IsOK());
EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("in-memory address marker"));
}

#endif // !defined(DISABLE_SPARSE_TENSORS)

// Defense-in-depth: GetExtDataFromTensorProto must reject absolute external paths even when
// called with an empty model_path (e.g. from training checkpoint or custom-op init paths).
// Previously, ValidateExternalDataPath was only invoked from Graph::ConvertInitializersIntoOrtValues,
// so direct callers of GetExtDataFromTensorProto could load arbitrary files.
TEST(GetExtDataFromTensorProtoTest, RejectsAbsoluteExternalPathWithEmptyModelPath) {
ONNX_NAMESPACE::TensorProto tensor_proto;
tensor_proto.set_name("abs_external");
tensor_proto.set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
tensor_proto.add_dims(2);
tensor_proto.set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);

auto* loc = tensor_proto.add_external_data();
loc->set_key("location");
#ifdef _WIN32
loc->set_value("C:\\data.bin");
#else
loc->set_value("/etc/passwd");
#endif

auto* off = tensor_proto.add_external_data();
off->set_key("offset");
off->set_value("0");

auto* len = tensor_proto.add_external_data();
len->set_key("length");
len->set_value(std::to_string(2 * sizeof(float)));

OrtValue value;
Status status = utils::GetExtDataFromTensorProto(Env::Default(), {}, tensor_proto, value);
ASSERT_FALSE(status.IsOK()) << "Absolute external path must be rejected even with empty model_path.";
EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed"));
}

// Defense-in-depth: GetExtDataFromTensorProto must reject directory-escaping external paths even
// when the caller passes a non-empty model_path. This guards callers outside Graph::Resolve.
TEST(GetExtDataFromTensorProtoTest, RejectsEscapingExternalPath) {
ONNX_NAMESPACE::TensorProto tensor_proto;
tensor_proto.set_name("escape_external");
tensor_proto.set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
tensor_proto.add_dims(2);
tensor_proto.set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL);

auto* loc = tensor_proto.add_external_data();
loc->set_key("location");
loc->set_value("../escape.bin");

auto* off = tensor_proto.add_external_data();
off->set_key("offset");
off->set_value("0");

auto* len = tensor_proto.add_external_data();
len->set_key("length");
len->set_value(std::to_string(2 * sizeof(float)));

OrtValue value;
// Pass a synthetic model_path so the validator has a model directory to compare against.
std::filesystem::path model_path = std::filesystem::temp_directory_path() / "sub" / "model.onnx";
Status status = utils::GetExtDataFromTensorProto(Env::Default(), model_path, tensor_proto, value);
ASSERT_FALSE(status.IsOK()) << "Directory-escaping external path must be rejected.";
EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes"));
}

TEST(TensorProtoUtilsTest, GetNodeProtoLayeringAnnotation) {
// Case 1: Annotation exists
{
Expand Down
Loading
Loading