From 084e3088ed8614c4876f177cb6da3b6a2aa7f87f Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Thu, 7 May 2026 18:12:29 -0700 Subject: [PATCH 01/13] First draft: validate sparse tensor external files (values and indices) --- .../core/framework/tensorprotoutils.cc | 14 ++ .../test/framework/tensorutils_test.cc | 125 ++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 360726d780a17..1353465286e9b 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2056,6 +2056,18 @@ 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. +static Status ValidateExternalDataForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, + const std::filesystem::path& model_path) { + if (!utils::HasExternalDataInFile(tensor_proto)) { + return Status::OK(); + } + std::unique_ptr 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()); +} + static Status CopySparseData(const std::string& name, int64_t nnz_elements, const ONNX_NAMESPACE::TensorProto& indices, @@ -2071,6 +2083,7 @@ static Status CopySparseData(const std::string& name, std::vector unpack_buffer; gsl::span indices_data; const bool needs_unpack = utils::HasRawData(indices) || utils::HasExternalData(indices); + ORT_RETURN_IF_ERROR(ValidateExternalDataForTensor(indices, model_path)); switch (indices.data_type()) { case ONNX_NAMESPACE::TensorProto_DataType_INT64: if (needs_unpack) { @@ -2291,6 +2304,7 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT std::string dense_data_storage(SafeInt(dense_elements) * element_size, 0); if (nnz_elements > 0) { // need to read in sparse data first as it could be in a type specific field, in raw data, or in external data + ORT_RETURN_IF_ERROR(ValidateExternalDataForTensor(sparse_values, model_path)); std::vector values_data; ORT_RETURN_IF_ERROR(UnpackInitializerData(sparse_values, model_path, values_data)); ORT_RETURN_IF_NOT(values_data.size() == SafeInt(nnz_elements) * element_size, diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 880208960a63c..e47f94f4147f4 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -715,6 +715,131 @@ TEST_F(PathValidationTest, ValidateExternalDataPathEmptyModelPathWithSymlinkOuts EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("escapes working directory")); } +#if !defined(DISABLE_SPARSE_TENSORS) +// Regression test: SparseTensorProtoToDenseTensorProto must reject external_data paths +// that escape the model directory (path traversal via "../" in location). +TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) { + // Create model directory and a "secret" file outside it. + auto model_dir = base_dir_ / "model_dir"; + std::filesystem::create_directories(model_dir); + + // Write known float data to a file outside the model directory. + auto secret_file = base_dir_ / "secret.txt"; + { + std::ofstream ofs(secret_file, std::ios::binary); + float secret_data[] = {42.0f, 99.0f}; + ofs.write(reinterpret_cast(secret_data), sizeof(secret_data)); + } + + // Construct a SparseTensorProto whose values use external data with a path-traversal location. + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(4); // dense shape: [4] + + // Values tensor: 2 non-zero float values stored in external file. + auto* values = sparse.mutable_values(); + values->set_name("sparse_test"); + values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT); + values->add_dims(2); // 2 non-zero elements + values->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL); + + auto* loc = values->add_external_data(); + loc->set_key("location"); + loc->set_value("../secret.txt"); // path traversal! + + auto* len_entry = values->add_external_data(); + len_entry->set_key("length"); + len_entry->set_value(std::to_string(2 * sizeof(float))); + + // Indices: positions 0 and 1 in the dense tensor. + 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); + + // Attempt to convert — this should fail with a path validation error. + ONNX_NAMESPACE::TensorProto dense; + std::filesystem::path model_path = model_dir / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject path-traversal " + "in values external_data location, but it succeeded (reading " + "arbitrary file outside model directory)."; + EXPECT_THAT(status.ErrorMessage(), + ::testing::AnyOf(::testing::HasSubstr("escapes"), + ::testing::HasSubstr("External data path"))); +} + +// Same as above but for path traversal in the indices external data. +// Note: The indices path also has a pre-existing issue where it checks raw_data().size() +// even when data_location is EXTERNAL. This test verifies the path traversal is blocked +// before that check runs (once the fix is in place). +TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) { + auto model_dir = base_dir_ / "model_dir"; + std::filesystem::create_directories(model_dir); + + // Write indices data (2 x int64) to a file outside the model directory. + auto secret_file = base_dir_ / "indices_secret.bin"; + { + std::ofstream ofs(secret_file, std::ios::binary); + int64_t idx_data[] = {0, 1}; + ofs.write(reinterpret_cast(idx_data), sizeof(idx_data)); + } + + // Also need a valid values file inside the model directory. + auto values_file = model_dir / "values.bin"; + { + std::ofstream ofs(values_file, std::ios::binary); + float val_data[] = {1.0f, 2.0f}; + ofs.write(reinterpret_cast(val_data), sizeof(val_data)); + } + + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(4); + + // Values: legitimate external data within model directory. + auto* values = sparse.mutable_values(); + values->set_name("sparse_idx_test"); + values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT); + values->add_dims(2); + values->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL); + + auto* val_loc = values->add_external_data(); + val_loc->set_key("location"); + val_loc->set_value("values.bin"); + + auto* val_len = values->add_external_data(); + val_len->set_key("length"); + val_len->set_value(std::to_string(2 * sizeof(float))); + + // Indices: external data with path traversal. + 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* idx_loc = indices->add_external_data(); + idx_loc->set_key("location"); + idx_loc->set_value("../indices_secret.bin"); // path traversal! + + auto* idx_len = indices->add_external_data(); + idx_len->set_key("length"); + idx_len->set_value(std::to_string(2 * sizeof(int64_t))); + + // Set raw_data to expected size so the pre-existing size check passes. + // This simulates a malicious model that also sets raw_data size to match. + indices->mutable_raw_data()->resize(2 * sizeof(int64_t), '\0'); + + ONNX_NAMESPACE::TensorProto dense; + std::filesystem::path model_path = model_dir / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject path-traversal " + "in indices external_data location, but it succeeded."; + EXPECT_THAT(status.ErrorMessage(), + ::testing::AnyOf(::testing::HasSubstr("escapes"), + ::testing::HasSubstr("External data path"))); +} +#endif // !defined(DISABLE_SPARSE_TENSORS) + TEST(TensorProtoUtilsTest, GetNodeProtoLayeringAnnotation) { // Case 1: Annotation exists { From ed72a1939f22dfdadf42c941db336522690911fe Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 8 May 2026 11:27:41 -0700 Subject: [PATCH 02/13] Rename function --- onnxruntime/core/framework/tensorprotoutils.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 1353465286e9b..f953c5e388a69 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2058,8 +2058,8 @@ 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. -static Status ValidateExternalDataForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, - const std::filesystem::path& model_path) { +static Status ValidateExternalDataPathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, + const std::filesystem::path& model_path) { if (!utils::HasExternalDataInFile(tensor_proto)) { return Status::OK(); } @@ -2083,7 +2083,7 @@ static Status CopySparseData(const std::string& name, std::vector unpack_buffer; gsl::span indices_data; const bool needs_unpack = utils::HasRawData(indices) || utils::HasExternalData(indices); - ORT_RETURN_IF_ERROR(ValidateExternalDataForTensor(indices, model_path)); + ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(indices, model_path)); switch (indices.data_type()) { case ONNX_NAMESPACE::TensorProto_DataType_INT64: if (needs_unpack) { @@ -2304,7 +2304,7 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT std::string dense_data_storage(SafeInt(dense_elements) * element_size, 0); if (nnz_elements > 0) { // need to read in sparse data first as it could be in a type specific field, in raw data, or in external data - ORT_RETURN_IF_ERROR(ValidateExternalDataForTensor(sparse_values, model_path)); + ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(sparse_values, model_path)); std::vector values_data; ORT_RETURN_IF_ERROR(UnpackInitializerData(sparse_values, model_path, values_data)); ORT_RETURN_IF_NOT(values_data.size() == SafeInt(nnz_elements) * element_size, From 157b65f6247e60b59f294737a1e668bab80c60bb Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 8 May 2026 11:31:27 -0700 Subject: [PATCH 03/13] Update func comment --- onnxruntime/core/framework/tensorprotoutils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index f953c5e388a69..f811c26b3bef6 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2057,7 +2057,7 @@ 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. +// Validates that a TensorProto's external data path does not escape the model directory and that the file exists. static Status ValidateExternalDataPathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, const std::filesystem::path& model_path) { if (!utils::HasExternalDataInFile(tensor_proto)) { From b0cc4575d17841b0dccea716e3e9d77dfcbe4a3a Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 8 May 2026 12:53:41 -0700 Subject: [PATCH 04/13] Address review comments --- onnxruntime/core/framework/tensorprotoutils.cc | 4 +++- onnxruntime/test/framework/tensorutils_test.cc | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index f811c26b3bef6..bfce4c0f6b096 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2057,7 +2057,9 @@ 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 and that the file exists. +// 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 external data files. static Status ValidateExternalDataPathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, const std::filesystem::path& model_path) { if (!utils::HasExternalDataInFile(tensor_proto)) { diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index e47f94f4147f4..9fa027aeab2d6 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -721,14 +721,18 @@ TEST_F(PathValidationTest, ValidateExternalDataPathEmptyModelPathWithSymlinkOuts TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) { // Create model directory and a "secret" file outside it. auto model_dir = base_dir_ / "model_dir"; - std::filesystem::create_directories(model_dir); + std::error_code ec; + std::filesystem::create_directories(model_dir, ec); + ASSERT_FALSE(ec) << "Failed to create model_dir: " << ec.message(); // Write known float data to a file outside the model directory. - auto secret_file = base_dir_ / "secret.txt"; + auto secret_file = base_dir_ / "secret.bin"; { std::ofstream ofs(secret_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << secret_file; float secret_data[] = {42.0f, 99.0f}; ofs.write(reinterpret_cast(secret_data), sizeof(secret_data)); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << secret_file; } // Construct a SparseTensorProto whose values use external data with a path-traversal location. @@ -744,7 +748,7 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) auto* loc = values->add_external_data(); loc->set_key("location"); - loc->set_value("../secret.txt"); // path traversal! + loc->set_value("../secret.bin"); // path traversal! auto* len_entry = values->add_external_data(); len_entry->set_key("length"); @@ -775,22 +779,28 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) // before that check runs (once the fix is in place). TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) { auto model_dir = base_dir_ / "model_dir"; - std::filesystem::create_directories(model_dir); + std::error_code ec; + std::filesystem::create_directories(model_dir, ec); + ASSERT_FALSE(ec) << "Failed to create model_dir: " << ec.message(); // Write indices data (2 x int64) to a file outside the model directory. auto secret_file = base_dir_ / "indices_secret.bin"; { std::ofstream ofs(secret_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << secret_file; int64_t idx_data[] = {0, 1}; ofs.write(reinterpret_cast(idx_data), sizeof(idx_data)); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << secret_file; } // Also need a valid values file inside the model directory. auto values_file = model_dir / "values.bin"; { std::ofstream ofs(values_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << values_file; float val_data[] = {1.0f, 2.0f}; ofs.write(reinterpret_cast(val_data), sizeof(val_data)); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << values_file; } ONNX_NAMESPACE::SparseTensorProto sparse; From de4828f6a214edf1fd3cde3075f66d1558a5251e Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 09:59:48 -0700 Subject: [PATCH 05/13] Fix sparse tensor indices size check for external data The CopySparseData function checked indices.raw_data().size() before unpacking, which is always 0 when data_location is EXTERNAL (data is in an external file, not in raw_data). This caused legitimate sparse tensors with external indices to always fail validation. Fix by moving UnpackInitializerData before the size check and validating unpack_buffer.size() instead, matching the pattern already used for sparse values. This works correctly for both inline raw_data and external data sources. Also remove the workaround in the indices path-traversal test that stuffed raw_data to bypass the broken check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/framework/tensorprotoutils.cc | 24 +++++++++---------- .../test/framework/tensorutils_test.cc | 7 ------ 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index bfce4c0f6b096..0dab049716d05 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2089,10 +2089,10 @@ static Status CopySparseData(const std::string& name, switch (indices.data_type()) { case ONNX_NAMESPACE::TensorProto_DataType_INT64: if (needs_unpack) { - ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int64_t), - "Sparse tensor: ", name, " indices raw data size does not match expected: ", - indices_elements * sizeof(int64_t)); ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); + ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int64_t), + "Sparse tensor: ", name, " indices data size does not match expected: ", + indices_elements * sizeof(int64_t)); indices_data = ReinterpretAsSpan(gsl::make_span(unpack_buffer)); } else { ORT_RETURN_IF_NOT(indices.int64_data_size() == indices_elements, @@ -2103,10 +2103,10 @@ static Status CopySparseData(const std::string& name, break; case ONNX_NAMESPACE::TensorProto_DataType_INT32: { if (needs_unpack) { - ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int32_t), - "Sparse tensor: ", name, " indices raw data size does not match expected: ", - indices_elements * sizeof(int32_t)); ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); + ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int32_t), + "Sparse tensor: ", name, " indices data size does not match expected: ", + indices_elements * sizeof(int32_t)); auto int32_span = ReinterpretAsSpan(gsl::make_span(unpack_buffer)); indices_values.insert(indices_values.cend(), int32_span.begin(), int32_span.end()); unpack_buffer.clear(); @@ -2122,10 +2122,10 @@ static Status CopySparseData(const std::string& name, } case ONNX_NAMESPACE::TensorProto_DataType_INT16: { if (needs_unpack) { - ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int16_t), - "Sparse tensor: ", name, " indices raw data size does not match expected: ", - indices_elements * sizeof(int16_t)); ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); + ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int16_t), + "Sparse tensor: ", name, " indices data size does not match expected: ", + indices_elements * sizeof(int16_t)); auto int16_span = ReinterpretAsSpan(gsl::make_span(unpack_buffer)); indices_values.insert(indices_values.cend(), int16_span.begin(), int16_span.end()); unpack_buffer.clear(); @@ -2141,10 +2141,10 @@ static Status CopySparseData(const std::string& name, } case ONNX_NAMESPACE::TensorProto_DataType_INT8: { if (needs_unpack) { - ORT_RETURN_IF_NOT(indices.raw_data().size() == narrow(indices_elements), - "Sparse tensor: ", name, " indices raw data size does not match expected: ", - indices_elements * sizeof(int8_t)); ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); + ORT_RETURN_IF_NOT(unpack_buffer.size() == narrow(indices_elements), + "Sparse tensor: ", name, " indices data size does not match expected: ", + indices_elements * sizeof(int8_t)); auto int8_span = ReinterpretAsSpan(gsl::make_span(unpack_buffer)); indices_values.insert(indices_values.cend(), int8_span.begin(), int8_span.end()); unpack_buffer.clear(); diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 9fa027aeab2d6..5bb55c097e56e 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -774,9 +774,6 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) } // Same as above but for path traversal in the indices external data. -// Note: The indices path also has a pre-existing issue where it checks raw_data().size() -// even when data_location is EXTERNAL. This test verifies the path traversal is blocked -// before that check runs (once the fix is in place). TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) { auto model_dir = base_dir_ / "model_dir"; std::error_code ec; @@ -835,10 +832,6 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) idx_len->set_key("length"); idx_len->set_value(std::to_string(2 * sizeof(int64_t))); - // Set raw_data to expected size so the pre-existing size check passes. - // This simulates a malicious model that also sets raw_data size to match. - indices->mutable_raw_data()->resize(2 * sizeof(int64_t), '\0'); - ONNX_NAMESPACE::TensorProto dense; std::filesystem::path model_path = model_dir / "model.onnx"; Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); From 917987f3eed9b9318c1561dae68d8b32ae160c82 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 11:42:34 -0700 Subject: [PATCH 06/13] Add positive tests for sparse tensors with external data --- .../test/framework/sparse_kernels_test.cc | 278 ++++++++++++++++++ 1 file changed, 278 insertions(+) diff --git a/onnxruntime/test/framework/sparse_kernels_test.cc b/onnxruntime/test/framework/sparse_kernels_test.cc index 59ec8f51b4f4e..9efaed8ac7bd6 100644 --- a/onnxruntime/test/framework/sparse_kernels_test.cc +++ b/onnxruntime/test/framework/sparse_kernels_test.cc @@ -2539,6 +2539,284 @@ TEST(SparseTensorConversionTests, SparseCooToDense_2DRowOutOfRange) { EXPECT_THAT(status.ErrorMessage(), testing::HasSubstr("Invalid COO 2D index")); } +// Positive tests for SparseTensorProtoToDenseTensorProto with external data. +// These verify end-to-end conversion succeeds when values and/or indices are stored +// in legitimate external files within the model directory. + +// Helper: write data to a temp file and configure a TensorProto to reference it as external data. +// The file is created in the current working directory using CreateTestFile. +// The ScopedFileDeleter is assigned immediately after file creation to ensure cleanup on any failure. +template +static void SetupExternalDataTensor(TensorProto_DataType type, + const std::vector& data, + PathString& filename, + TensorProto& tensor_proto, + ScopedFileDeleter& file_deleter) { + size_t size_in_bytes = data.size() * sizeof(T); + std::vector le_data(size_in_bytes); + + auto src_span = gsl::make_span(data.data(), data.size()); + auto dst_span = gsl::make_span(le_data.data(), le_data.size()); + ASSERT_STATUS_OK(onnxruntime::utils::WriteLittleEndian(src_span, dst_span)); + + FILE* fp; + CreateTestFile(fp, filename); + file_deleter = ScopedFileDeleter(filename); + ASSERT_EQ(size_in_bytes, fwrite(le_data.data(), 1, size_in_bytes, fp)); + ASSERT_EQ(0, fclose(fp)); + + tensor_proto.set_data_type(type); + tensor_proto.set_data_location(TensorProto_DataLocation_EXTERNAL); + + auto* loc = tensor_proto.mutable_external_data()->Add(); + loc->set_key("location"); + loc->set_value(ToUTF8String(filename)); + + auto* len = tensor_proto.mutable_external_data()->Add(); + len->set_key("length"); + len->set_value(std::to_string(size_in_bytes)); +} + +// External values + inline indices (INT64), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_ExternalValues_InlineIndices) { + // Dense shape [2, 3] = 6 elements. + // NNZ=3 values at linear indices [0, 2, 5]. + // Expected dense: [1.0, 0, 2.0, 0, 0, 3.0] + std::vector values = {1.0f, 2.0f, 3.0f}; + PathString values_file(ORT_TSTR("ext_val_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(2); + sparse.add_dims(3); + + ScopedFileDeleter values_deleter; + SetupExternalDataTensor(TensorProto_DataType_FLOAT, values, values_file, *sparse.mutable_values(), + values_deleter); + sparse.mutable_values()->set_name("ext_values_test"); + sparse.mutable_values()->add_dims(3); // NNZ + + auto* indices = sparse.mutable_indices(); + indices->set_data_type(TensorProto_DataType_INT64); + indices->add_dims(3); + indices->add_int64_data(0); + indices->add_int64_data(2); + indices->add_int64_data(5); + + // model_path in CWD so external files are within the model directory + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + ASSERT_EQ(dense.dims_size(), 2); + EXPECT_EQ(dense.dims(0), 2); + EXPECT_EQ(dense.dims(1), 3); + + std::vector unpacked(6); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {1.0f, 0.0f, 2.0f, 0.0f, 0.0f, 3.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Inline values + external indices (INT64), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_InlineValues_ExternalIndicesInt64) { + // Dense shape [4] = 4 elements. + // NNZ=2 at indices [1, 3]. + // Expected dense: [0, 10.0, 0, 20.0] + std::vector indices_data = {1, 3}; + PathString indices_file(ORT_TSTR("ext_idx_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(4); + + auto* values = sparse.mutable_values(); + values->set_name("ext_indices_test"); + values->set_data_type(TensorProto_DataType_FLOAT); + values->add_dims(2); + values->add_float_data(10.0f); + values->add_float_data(20.0f); + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT64, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + std::vector unpacked(4); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {0.0f, 10.0f, 0.0f, 20.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Inline values + external indices (INT32), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_InlineValues_ExternalIndicesInt32) { + std::vector indices_data = {0, 3}; + PathString indices_file(ORT_TSTR("ext_i32_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(2); + sparse.add_dims(2); + + auto* values = sparse.mutable_values(); + values->set_name("ext_int32_idx_test"); + values->set_data_type(TensorProto_DataType_FLOAT); + values->add_dims(2); + values->add_float_data(5.0f); + values->add_float_data(6.0f); + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT32, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + std::vector unpacked(4); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {5.0f, 0.0f, 0.0f, 6.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Inline values + external indices (INT16), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_InlineValues_ExternalIndicesInt16) { + std::vector indices_data = {1, 2}; + PathString indices_file(ORT_TSTR("ext_i16_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(4); + + auto* values = sparse.mutable_values(); + values->set_name("ext_int16_idx_test"); + values->set_data_type(TensorProto_DataType_FLOAT); + values->add_dims(2); + values->add_float_data(7.0f); + values->add_float_data(8.0f); + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT16, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + std::vector unpacked(4); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {0.0f, 7.0f, 8.0f, 0.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Inline values + external indices (INT8), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_InlineValues_ExternalIndicesInt8) { + std::vector indices_data = {0, 2}; + PathString indices_file(ORT_TSTR("ext_i8_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(3); + + auto* values = sparse.mutable_values(); + values->set_name("ext_int8_idx_test"); + values->set_data_type(TensorProto_DataType_FLOAT); + values->add_dims(2); + values->add_float_data(9.0f); + values->add_float_data(11.0f); + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT8, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + std::vector unpacked(3); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {9.0f, 0.0f, 11.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Both external values and external indices (INT64), rank-1 COO. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_ExternalValues_ExternalIndicesInt64) { + // Dense shape [3, 2] = 6 elements. + // NNZ=2 at linear indices [1, 4]. + // Expected dense: [0, 100.0, 0, 0, 200.0, 0] + std::vector values_data = {100.0f, 200.0f}; + std::vector indices_data = {1, 4}; + PathString values_file(ORT_TSTR("ext_bv_XXXXXX")); + PathString indices_file(ORT_TSTR("ext_bi_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(3); + sparse.add_dims(2); + + ScopedFileDeleter values_deleter; + SetupExternalDataTensor(TensorProto_DataType_FLOAT, values_data, values_file, *sparse.mutable_values(), + values_deleter); + sparse.mutable_values()->set_name("ext_both_test"); + sparse.mutable_values()->add_dims(2); + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT64, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + ASSERT_EQ(dense.dims_size(), 2); + EXPECT_EQ(dense.dims(0), 3); + EXPECT_EQ(dense.dims(1), 2); + + std::vector unpacked(6); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {0.0f, 100.0f, 0.0f, 0.0f, 200.0f, 0.0f}; + EXPECT_EQ(unpacked, expected); +} + +// Both external values and external indices (INT64), rank-2 COO indices. +TEST(SparseTensorConversionTests, SparseTensorProtoToDense_ExternalValues_ExternalIndicesInt64_Rank2) { + // Dense shape [3, 3] = 9 elements. + // NNZ=2 with 2D indices: [[0, 2], [2, 0]] -> positions (0,2)=2, (2,0)=6. + // Expected dense: [0, 0, 50.0, 0, 0, 0, 60.0, 0, 0] + std::vector values_data = {50.0f, 60.0f}; + // Rank-2 indices: flattened as [row0, col0, row1, col1] + std::vector indices_data = {0, 2, 2, 0}; + PathString values_file(ORT_TSTR("ext_r2v_XXXXXX")); + PathString indices_file(ORT_TSTR("ext_r2i_XXXXXX")); + + SparseTensorProto sparse; + sparse.add_dims(3); + sparse.add_dims(3); + + ScopedFileDeleter values_deleter; + SetupExternalDataTensor(TensorProto_DataType_FLOAT, values_data, values_file, *sparse.mutable_values(), + values_deleter); + sparse.mutable_values()->set_name("ext_rank2_test"); + sparse.mutable_values()->add_dims(2); // NNZ + + ScopedFileDeleter indices_deleter; + SetupExternalDataTensor(TensorProto_DataType_INT64, indices_data, indices_file, + *sparse.mutable_indices(), indices_deleter); + sparse.mutable_indices()->add_dims(2); // NNZ + sparse.mutable_indices()->add_dims(2); // rank of dense tensor + + std::filesystem::path model_path = std::filesystem::current_path() / "model.onnx"; + TensorProto dense; + ASSERT_STATUS_OK(utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense)); + + std::vector unpacked(9); + ASSERT_STATUS_OK(utils::UnpackTensor(dense, model_path, unpacked.data(), unpacked.size())); + std::vector expected = {0.0f, 0.0f, 50.0f, 0.0f, 0.0f, 0.0f, 60.0f, 0.0f, 0.0f}; + EXPECT_EQ(unpacked, expected); +} + #endif // !defined(DISABLE_SPARSE_TENSORS) } // namespace test } // namespace onnxruntime From 4785b3eea858855de25cb201de182196577ffd29 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 11:43:02 -0700 Subject: [PATCH 07/13] Add path validation tests (reject absolute paths) for sparse tensors with external data --- .../test/framework/tensorutils_test.cc | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 5bb55c097e56e..a6a0d4365ae66 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -841,6 +841,111 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) ::testing::AnyOf(::testing::HasSubstr("escapes"), ::testing::HasSubstr("External data path"))); } + +// Regression test: SparseTensorProtoToDenseTensorProto must reject absolute paths +// in values external_data location. +TEST_F(PathValidationTest, SparseTensorExternalDataAbsolutePathBlocked_Values) { + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(4); + + auto* values = sparse.mutable_values(); + values->set_name("abs_path_test"); + 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("/data.bin"); // absolute path + + auto* len_entry = values->add_external_data(); + len_entry->set_key("length"); + len_entry->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; + std::filesystem::path model_path = base_dir_ / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject absolute path " + "in values external_data location."; + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); + +#ifdef _WIN32 + // Also verify Windows-style absolute path. + loc->set_value("C:\\data.bin"); + status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject Windows absolute path " + "in values external_data location."; + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); +#endif +} + +// Regression test: SparseTensorProtoToDenseTensorProto must reject absolute paths +// in indices external_data location. +TEST_F(PathValidationTest, SparseTensorExternalDataAbsolutePathBlocked_Indices) { + // Create a valid values file inside base_dir_ so values validation passes. + auto values_file = base_dir_ / "values.bin"; + { + std::ofstream ofs(values_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << values_file; + float val_data[] = {1.0f, 2.0f}; + ofs.write(reinterpret_cast(val_data), sizeof(val_data)); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << values_file; + } + + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(4); + + // Values: legitimate external data within base_dir_. + auto* values = sparse.mutable_values(); + values->set_name("abs_path_idx_test"); + values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT); + values->add_dims(2); + values->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL); + + auto* val_loc = values->add_external_data(); + val_loc->set_key("location"); + val_loc->set_value("values.bin"); + + auto* val_len = values->add_external_data(); + val_len->set_key("length"); + val_len->set_value(std::to_string(2 * sizeof(float))); + + // Indices: external data with absolute path. + 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* idx_loc = indices->add_external_data(); + idx_loc->set_key("location"); + idx_loc->set_value("/data.bin"); // absolute path + + auto* idx_len = indices->add_external_data(); + idx_len->set_key("length"); + idx_len->set_value(std::to_string(2 * sizeof(int64_t))); + + ONNX_NAMESPACE::TensorProto dense; + std::filesystem::path model_path = base_dir_ / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject absolute path " + "in indices external_data location."; + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); + +#ifdef _WIN32 + idx_loc->set_value("C:\\data.bin"); + status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject Windows absolute path " + "in indices external_data location."; + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); +#endif +} + #endif // !defined(DISABLE_SPARSE_TENSORS) TEST(TensorProtoUtilsTest, GetNodeProtoLayeringAnnotation) { From 9c4e1caf33a3c954a381b7cc6026247bfce70e3c Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 11:50:12 -0700 Subject: [PATCH 08/13] Move check outside to account for nnz=0 --- onnxruntime/core/framework/tensorprotoutils.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 0dab049716d05..d97dd90ef356b 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2304,9 +2304,10 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT // by putting the data into a std::string we can avoid a copy as set_raw_data can do a std::move // into the TensorProto. std::string dense_data_storage(SafeInt(dense_elements) * element_size, 0); + // Validate external data path unconditionally (defense-in-depth even when NNZ=0). + ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(sparse_values, model_path)); if (nnz_elements > 0) { // need to read in sparse data first as it could be in a type specific field, in raw data, or in external data - ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(sparse_values, model_path)); std::vector values_data; ORT_RETURN_IF_ERROR(UnpackInitializerData(sparse_values, model_path, values_data)); ORT_RETURN_IF_NOT(values_data.size() == SafeInt(nnz_elements) * element_size, From be9a8c954208090dfbda99d2de87c53f584783e0 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 13:08:05 -0700 Subject: [PATCH 09/13] Address copilot review comments: move validation checks earlier to catch it upfront --- onnxruntime/core/framework/tensorprotoutils.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 630080dadd767..c01b5a28fb5f3 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2126,7 +2126,6 @@ static Status CopySparseData(const std::string& name, std::vector unpack_buffer; gsl::span indices_data; const bool needs_unpack = utils::HasRawData(indices) || utils::HasExternalData(indices); - ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(indices, model_path)); switch (indices.data_type()) { case ONNX_NAMESPACE::TensorProto_DataType_INT64: if (needs_unpack) { @@ -2333,6 +2332,12 @@ 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)); + if (dense_elements == 0) { // if there are no elements in the dense tensor, we can return early with an empty tensor proto return status; @@ -2345,8 +2350,6 @@ common::Status SparseTensorProtoToDenseTensorProto(const ONNX_NAMESPACE::SparseT // by putting the data into a std::string we can avoid a copy as set_raw_data can do a std::move // into the TensorProto. std::string dense_data_storage(SafeInt(dense_elements) * element_size, 0); - // Validate external data path unconditionally (defense-in-depth even when NNZ=0). - ORT_RETURN_IF_ERROR(ValidateExternalDataPathForTensor(sparse_values, model_path)); if (nnz_elements > 0) { // need to read in sparse data first as it could be in a type specific field, in raw data, or in external data std::vector values_data; From 671f81c2b2fc31f9f02e40a6f8fdd2711fc743cd Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 13:41:39 -0700 Subject: [PATCH 10/13] Add tets for zero nnz case --- .../test/framework/tensorutils_test.cc | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 83b151b8590b0..3817b3ba02f61 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -1065,6 +1065,72 @@ TEST_F(PathValidationTest, SparseTensorExternalDataAbsolutePathBlocked_Indices) EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("Absolute path not allowed")); #endif } + +// Regression test: validation must still reject escaping paths for zero-element dense tensors, +// which previously returned early before path validation ran. +TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroDenseElements) { + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(0); // dense shape [0] → dense_elements == 0 + + auto* values = sparse.mutable_values(); + values->set_name("zero_dense_test"); + values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT); + values->add_dims(0); // NNZ=0 + values->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL); + + auto* loc = values->add_external_data(); + loc->set_key("location"); + loc->set_value("../secret.bin"); // path traversal + + auto* len_entry = values->add_external_data(); + len_entry->set_key("length"); + len_entry->set_value("0"); + + auto* indices = sparse.mutable_indices(); + indices->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_INT64); + indices->add_dims(0); + + ONNX_NAMESPACE::TensorProto dense; + std::filesystem::path model_path = base_dir_ / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "Should reject path-traversal in values even when dense_elements == 0."; + EXPECT_THAT(status.ErrorMessage(), + ::testing::AnyOf(::testing::HasSubstr("escapes"), + ::testing::HasSubstr("External data path"))); +} + +// Regression test: validation must reject escaping paths in indices even when NNZ == 0. +TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroNNZ) { + ONNX_NAMESPACE::SparseTensorProto sparse; + sparse.add_dims(4); // dense shape [4] → non-zero dense_elements + + auto* values = sparse.mutable_values(); + values->set_name("zero_nnz_test"); + values->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_FLOAT); + values->add_dims(0); // NNZ=0 + + auto* indices = sparse.mutable_indices(); + indices->set_data_type(ONNX_NAMESPACE::TensorProto_DataType_INT64); + indices->add_dims(0); + indices->set_data_location(ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL); + + auto* idx_loc = indices->add_external_data(); + idx_loc->set_key("location"); + idx_loc->set_value("../indices_secret.bin"); // path traversal + + auto* idx_len = indices->add_external_data(); + idx_len->set_key("length"); + idx_len->set_value("0"); + + ONNX_NAMESPACE::TensorProto dense; + std::filesystem::path model_path = base_dir_ / "model.onnx"; + Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); + ASSERT_FALSE(status.IsOK()) << "Should reject path-traversal in indices even when NNZ == 0."; + EXPECT_THAT(status.ErrorMessage(), + ::testing::AnyOf(::testing::HasSubstr("escapes"), + ::testing::HasSubstr("External data path"))); +} + #endif // !defined(DISABLE_SPARSE_TENSORS) TEST(TensorProtoUtilsTest, GetNodeProtoLayeringAnnotation) { From dae7767a53a6941f42bafbeedcf6ab1bda294064 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 14:56:02 -0700 Subject: [PATCH 11/13] Fix HasExternalDataInFile bypass and strengthen zero-element tests - Replace HasExternalDataInFile() gate in ValidateExternalDataPathForTensor with direct data_location == EXTERNAL check + parsed ExternalDataInfo to prevent bypasses via UNDEFINED data_type or duplicate location entries with in-memory markers - Skip validation for in-memory markers (ORT_MEM_ADDR) by checking the parsed rel_path against the known marker constants - Update zero-element regression tests to create escaping files and assert specifically for 'escapes' error to ensure path-traversal detection (not just file-not-found) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/framework/tensorprotoutils.cc | 17 ++++++-- .../test/framework/tensorutils_test.cc | 40 +++++++++++++++---- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index c01b5a28fb5f3..30ae8528be2a1 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2100,15 +2100,26 @@ void MakeCpuTensorCopy(const Tensor& src_tensor, Tensor& dst_tensor) { // 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 external data files. +// Returns Status::OK() (no-op) for tensors that do not use file-based external data. +// Gates on data_location == EXTERNAL directly (not HasExternalDataInFile) to avoid bypasses +// via UNDEFINED data_type or duplicate location entries with in-memory markers. static Status ValidateExternalDataPathForTensor(const ONNX_NAMESPACE::TensorProto& tensor_proto, const std::filesystem::path& model_path) { - if (!utils::HasExternalDataInFile(tensor_proto)) { + if (tensor_proto.data_location() != ONNX_NAMESPACE::TensorProto_DataLocation_EXTERNAL) { return Status::OK(); } + std::unique_ptr 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()); + 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); } static Status CopySparseData(const std::string& name, diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 3817b3ba02f61..4da8fff8e8ffa 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -1069,6 +1069,20 @@ TEST_F(PathValidationTest, SparseTensorExternalDataAbsolutePathBlocked_Indices) // Regression test: validation must still reject escaping paths for zero-element dense tensors, // which previously returned early before path validation ran. TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroDenseElements) { + auto model_dir = base_dir_ / "model_dir"; + std::error_code ec; + std::filesystem::create_directories(model_dir, ec); + ASSERT_FALSE(ec) << "Failed to create model_dir: " << ec.message(); + + // Create the escaping file so that a "file not found" error would NOT be raised. + auto secret_file = base_dir_ / "secret.bin"; + { + std::ofstream ofs(secret_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << secret_file; + ofs.put('\0'); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << secret_file; + } + ONNX_NAMESPACE::SparseTensorProto sparse; sparse.add_dims(0); // dense shape [0] → dense_elements == 0 @@ -1091,16 +1105,28 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroDens indices->add_dims(0); ONNX_NAMESPACE::TensorProto dense; - std::filesystem::path model_path = base_dir_ / "model.onnx"; + std::filesystem::path model_path = model_dir / "model.onnx"; Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); ASSERT_FALSE(status.IsOK()) << "Should reject path-traversal in values even when dense_elements == 0."; - EXPECT_THAT(status.ErrorMessage(), - ::testing::AnyOf(::testing::HasSubstr("escapes"), - ::testing::HasSubstr("External data path"))); + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes")); } // Regression test: validation must reject escaping paths in indices even when NNZ == 0. TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroNNZ) { + auto model_dir = base_dir_ / "model_dir"; + std::error_code ec; + std::filesystem::create_directories(model_dir, ec); + ASSERT_FALSE(ec) << "Failed to create model_dir: " << ec.message(); + + // Create the escaping file so that a "file not found" error would NOT be raised. + auto secret_file = base_dir_ / "indices_secret.bin"; + { + std::ofstream ofs(secret_file, std::ios::binary); + ASSERT_TRUE(ofs.is_open()) << "Failed to open " << secret_file; + ofs.put('\0'); + ASSERT_TRUE(ofs.good()) << "Failed to write to " << secret_file; + } + ONNX_NAMESPACE::SparseTensorProto sparse; sparse.add_dims(4); // dense shape [4] → non-zero dense_elements @@ -1123,12 +1149,10 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_ZeroNNZ) idx_len->set_value("0"); ONNX_NAMESPACE::TensorProto dense; - std::filesystem::path model_path = base_dir_ / "model.onnx"; + std::filesystem::path model_path = model_dir / "model.onnx"; Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); ASSERT_FALSE(status.IsOK()) << "Should reject path-traversal in indices even when NNZ == 0."; - EXPECT_THAT(status.ErrorMessage(), - ::testing::AnyOf(::testing::HasSubstr("escapes"), - ::testing::HasSubstr("External data path"))); + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes")); } #endif // !defined(DISABLE_SPARSE_TENSORS) From 7cb85e3173a106c90201d0a984ddb6aaa98efce6 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 15:10:14 -0700 Subject: [PATCH 12/13] Add comment about why not using utils --- onnxruntime/core/framework/tensorprotoutils.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 30ae8528be2a1..080da1c533b29 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2101,10 +2101,12 @@ void MakeCpuTensorCopy(const Tensor& src_tensor, Tensor& dst_tensor) { // 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. -// Gates on data_location == EXTERNAL directly (not HasExternalDataInFile) to avoid bypasses -// via UNDEFINED data_type or duplicate location entries with in-memory markers. 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) { return Status::OK(); } From 79dae211fd82edee5a562d6bd57c90cefa28d420 Mon Sep 17 00:00:00 2001 From: adrianlizarraga Date: Fri, 15 May 2026 15:27:39 -0700 Subject: [PATCH 13/13] Restore raw_data().size() check for non-external indices --- .../core/framework/tensorprotoutils.cc | 23 +++++++++++++++++++ .../test/framework/tensorutils_test.cc | 8 ++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index 080da1c533b29..275fa837a7257 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -2142,6 +2142,14 @@ static Status CopySparseData(const std::string& name, switch (indices.data_type()) { case ONNX_NAMESPACE::TensorProto_DataType_INT64: if (needs_unpack) { + // For inline raw_data, validate size before unpacking to avoid a large allocation from a + // malformed tensor with small indices shape but oversized raw_data. For external data, + // raw_data is empty so we can only validate after unpacking. + if (!utils::HasExternalData(indices)) { + ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int64_t), + "Sparse tensor: ", name, " indices raw data size does not match expected: ", + indices_elements * sizeof(int64_t)); + } ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int64_t), "Sparse tensor: ", name, " indices data size does not match expected: ", @@ -2156,6 +2164,11 @@ static Status CopySparseData(const std::string& name, break; case ONNX_NAMESPACE::TensorProto_DataType_INT32: { if (needs_unpack) { + if (!utils::HasExternalData(indices)) { + ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int32_t), + "Sparse tensor: ", name, " indices raw data size does not match expected: ", + indices_elements * sizeof(int32_t)); + } ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int32_t), "Sparse tensor: ", name, " indices data size does not match expected: ", @@ -2175,6 +2188,11 @@ static Status CopySparseData(const std::string& name, } case ONNX_NAMESPACE::TensorProto_DataType_INT16: { if (needs_unpack) { + if (!utils::HasExternalData(indices)) { + ORT_RETURN_IF_NOT(indices.raw_data().size() == SafeInt(indices_elements) * sizeof(int16_t), + "Sparse tensor: ", name, " indices raw data size does not match expected: ", + indices_elements * sizeof(int16_t)); + } ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); ORT_RETURN_IF_NOT(unpack_buffer.size() == SafeInt(indices_elements) * sizeof(int16_t), "Sparse tensor: ", name, " indices data size does not match expected: ", @@ -2194,6 +2212,11 @@ static Status CopySparseData(const std::string& name, } case ONNX_NAMESPACE::TensorProto_DataType_INT8: { if (needs_unpack) { + if (!utils::HasExternalData(indices)) { + ORT_RETURN_IF_NOT(indices.raw_data().size() == narrow(indices_elements), + "Sparse tensor: ", name, " indices raw data size does not match expected: ", + indices_elements * sizeof(int8_t)); + } ORT_RETURN_IF_ERROR(UnpackInitializerData(indices, model_path, unpack_buffer)); ORT_RETURN_IF_NOT(unpack_buffer.size() == narrow(indices_elements), "Sparse tensor: ", name, " indices data size does not match expected: ", diff --git a/onnxruntime/test/framework/tensorutils_test.cc b/onnxruntime/test/framework/tensorutils_test.cc index 4da8fff8e8ffa..06cc3ea6ad8d2 100644 --- a/onnxruntime/test/framework/tensorutils_test.cc +++ b/onnxruntime/test/framework/tensorutils_test.cc @@ -888,9 +888,7 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Values) ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject path-traversal " "in values external_data location, but it succeeded (reading " "arbitrary file outside model directory)."; - EXPECT_THAT(status.ErrorMessage(), - ::testing::AnyOf(::testing::HasSubstr("escapes"), - ::testing::HasSubstr("External data path"))); + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes")); } // Same as above but for path traversal in the indices external data. @@ -957,9 +955,7 @@ TEST_F(PathValidationTest, SparseTensorExternalDataPathTraversalBlocked_Indices) Status status = utils::SparseTensorProtoToDenseTensorProto(sparse, model_path, dense); ASSERT_FALSE(status.IsOK()) << "SparseTensorProtoToDenseTensorProto should reject path-traversal " "in indices external_data location, but it succeeded."; - EXPECT_THAT(status.ErrorMessage(), - ::testing::AnyOf(::testing::HasSubstr("escapes"), - ::testing::HasSubstr("External data path"))); + EXPECT_THAT(status.ErrorMessage(), ::testing::HasSubstr("escapes")); } // Regression test: SparseTensorProtoToDenseTensorProto must reject absolute paths