Skip to content

Commit 50d04b9

Browse files
Copilottianleiwu
andauthored
Fix out-of-bounds read in CropBase: validate scale attribute length (microsoft#28399)
### Description `CropBase::ValidateInput` guarded `scale_[0]`/`scale_[1]` access with `!scale_.empty()` but never enforced `scale_.size() == 2`. A malformed model with a wrong-length `scale` attribute (e.g. 1 element) causes an out-of-bounds read in both validation and compute paths. **Changes:** - **`contrib_ops/cpu/crop.h`** — inside the `!scale_.empty()` branch, return `INVALID_ARGUMENT` early if `scale_.size() != 2` with a descriptive message before any indexing occurs. `Compute` is protected transitively since it calls `ValidateInput` first. - **`test/contrib_ops/crop_op_test.cc`** — added `Crop_Invalid_Scale_Size` test: passes a 1-element `scale` attribute and asserts the expected error is returned. ### Motivation and Context `scale_` is a user-supplied attribute read directly from the model. Without length validation a malformed model can trigger undefined behavior (out-of-bounds vector read) in both the CPU and CUDA `Crop` kernels (CUDA inherits `CropBase`). Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com> Co-authored-by: Tianlei Wu <tlwu@microsoft.com>
1 parent dcf8775 commit 50d04b9

2 files changed

Lines changed: 25 additions & 0 deletions

File tree

onnxruntime/contrib_ops/cpu/crop.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ class CropBase {
5353

5454
// scale = (height, width)
5555
if (!scale_.empty()) {
56+
if (scale_.size() != 2) {
57+
return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_ARGUMENT,
58+
"Attribute scale needs to be specified with two elements (height, width), got ",
59+
scale_.size());
60+
}
5661
int64_t bottomLimit = topBorder + scale_[0];
5762
int64_t rightLimit = leftBorder + scale_[1];
5863

onnxruntime/test/contrib_ops/crop_op_test.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "gtest/gtest.h"
55
#include "test/providers/provider_test_utils.h"
6+
#include "test/util/include/default_providers.h"
67

78
namespace onnxruntime {
89
namespace test {
@@ -30,5 +31,24 @@ TEST(CropOpTest, Crop_Scale) {
3031
test.Run(OpTester::ExpectResult::kExpectSuccess, "", {kTensorrtExecutionProvider});
3132
}
3233

34+
TEST(CropOpTest, Crop_Invalid_Scale_Size) {
35+
OpTester test("Crop", 1, onnxruntime::kOnnxDomain);
36+
test.AddInput<float>("x", {1, 1, 4, 4}, {1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0});
37+
38+
std::vector<int64_t> border{1, 1, 1, 1};
39+
test.AddAttribute("border", border);
40+
41+
std::vector<int64_t> scale{2}; // invalid: must be exactly 2 elements
42+
test.AddAttribute("scale", scale);
43+
44+
test.AddOutput<float>("y", {1, 1, 2, 2}, {6.0, 7.0, 10.0, 11.0});
45+
46+
std::vector<std::unique_ptr<IExecutionProvider>> execution_providers;
47+
execution_providers.push_back(DefaultCpuExecutionProvider());
48+
test.Run(OpTester::ExpectResult::kExpectFailure,
49+
"Attribute scale needs to be specified with two elements (height, width), got 1",
50+
{}, nullptr, &execution_providers);
51+
}
52+
3353
} // namespace test
3454
} // namespace onnxruntime

0 commit comments

Comments
 (0)