Skip to content

Commit dfc80a1

Browse files
committed
GH-50304: [C++][IPC] Reject negative sparse tensor shape and non-zero length
Validate dimension sizes and non_zero_length in GetSparseTensorMetadata so malformed IPC SparseTensor messages are rejected before the values flow into buffer-size computations. The now-redundant non-zero-length check in ReadSparseCSXIndex is removed in favour of this central guard.
1 parent e92d5d7 commit dfc80a1

3 files changed

Lines changed: 37 additions & 16 deletions

File tree

cpp/src/arrow/ipc/metadata_internal.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,9 @@ Status GetSparseTensorMetadata(const Buffer& metadata, std::shared_ptr<DataType>
15591559
auto dim = sparse_tensor->shape()->Get(i);
15601560

15611561
if (shape) {
1562+
if (dim->size() < 0) {
1563+
return Status::Invalid("Invalid sparse tensor dimension size: ", dim->size());
1564+
}
15621565
shape->push_back(dim->size());
15631566
}
15641567

@@ -1569,6 +1572,10 @@ Status GetSparseTensorMetadata(const Buffer& metadata, std::shared_ptr<DataType>
15691572
}
15701573

15711574
if (non_zero_length) {
1575+
if (sparse_tensor->non_zero_length() < 0) {
1576+
return Status::Invalid("Invalid sparse tensor non-zero length: ",
1577+
sparse_tensor->non_zero_length());
1578+
}
15721579
*non_zero_length = sparse_tensor->non_zero_length();
15731580
}
15741581

cpp/src/arrow/ipc/reader.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,9 +2339,6 @@ Result<std::shared_ptr<SparseIndex>> ReadSparseCSXIndex(
23392339
if (shape.size() != 2) {
23402340
return Status::Invalid("Invalid shape length for a sparse matrix");
23412341
}
2342-
if (non_zero_length < 0) {
2343-
return Status::Invalid("Invalid non-zero length for a sparse matrix");
2344-
}
23452342

23462343
auto* sparse_index = sparse_tensor->sparseIndex_as_SparseMatrixIndexCSX();
23472344

cpp/src/arrow/ipc/tensor_test.cc

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -510,17 +510,16 @@ namespace {
510510
// Build a SparseTensor IPC message carrying a CSF index with caller-controlled
511511
// buffer counts. This lets us exercise the reader's validation directly, since
512512
// such inconsistent indices can only be produced by hand-crafted flatbuffers.
513-
Result<std::shared_ptr<Message>> MakeCSFSparseTensorMessage(int num_dims,
514-
int num_indptr_buffers,
515-
int num_indices_buffers,
516-
int axis_order_size) {
513+
Result<std::shared_ptr<Message>> MakeCSFSparseTensorMessage(
514+
const std::vector<int64_t>& shape, int num_indptr_buffers, int num_indices_buffers,
515+
int axis_order_size, int64_t non_zero_length = 0) {
517516
flatbuffers::FlatBufferBuilder fbb;
518517

519518
auto value_type = flatbuf::CreateInt(fbb, 64, /*is_signed=*/true);
520519

521520
std::vector<flatbuffers::Offset<flatbuf::TensorDim>> dims;
522-
for (int i = 0; i < num_dims; ++i) {
523-
dims.push_back(flatbuf::CreateTensorDim(fbb, /*size=*/4, /*name=*/0));
521+
for (int64_t dim_size : shape) {
522+
dims.push_back(flatbuf::CreateTensorDim(fbb, dim_size, /*name=*/0));
524523
}
525524
auto fb_shape = fbb.CreateVector(dims);
526525

@@ -540,7 +539,7 @@ Result<std::shared_ptr<Message>> MakeCSFSparseTensorMessage(int num_dims,
540539

541540
flatbuf::Buffer data(0, 0);
542541
auto sparse_tensor = flatbuf::CreateSparseTensor(
543-
fbb, flatbuf::Type_Int, value_type.Union(), fb_shape, /*non_zero_length=*/0,
542+
fbb, flatbuf::Type_Int, value_type.Union(), fb_shape, non_zero_length,
544543
flatbuf::SparseTensorIndex::SparseTensorIndex_SparseTensorIndexCSF, csf.Union(),
545544
&data);
546545

@@ -560,28 +559,46 @@ Result<std::shared_ptr<Message>> MakeCSFSparseTensorMessage(int num_dims,
560559
TEST(TestSparseCSFIndex, RejectInconsistentBufferCounts) {
561560
// ndim == 1 is not a valid CSF index (it has no indptr buffers), and used to
562561
// reach SparseCSFIndex's constructor with an empty indptr vector.
563-
ASSERT_OK_AND_ASSIGN(
564-
auto message, MakeCSFSparseTensorMessage(/*num_dims=*/1, /*num_indptr_buffers=*/0,
565-
/*num_indices_buffers=*/1,
566-
/*axis_order_size=*/1));
562+
ASSERT_OK_AND_ASSIGN(auto message,
563+
MakeCSFSparseTensorMessage(/*shape=*/{4}, /*num_indptr_buffers=*/0,
564+
/*num_indices_buffers=*/1,
565+
/*axis_order_size=*/1));
567566
ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
568567

569568
// Too many indices buffers for the declared number of dimensions, which used
570569
// to write past the end of the fixed-size index vectors.
571570
ASSERT_OK_AND_ASSIGN(
572-
message, MakeCSFSparseTensorMessage(/*num_dims=*/2, /*num_indptr_buffers=*/1,
571+
message, MakeCSFSparseTensorMessage(/*shape=*/{4, 4}, /*num_indptr_buffers=*/1,
573572
/*num_indices_buffers=*/3,
574573
/*axis_order_size=*/2));
575574
ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
576575

577576
// axisOrder and indicesBuffers lengths disagree (out-of-bounds read).
578577
ASSERT_OK_AND_ASSIGN(
579-
message, MakeCSFSparseTensorMessage(/*num_dims=*/2, /*num_indptr_buffers=*/1,
578+
message, MakeCSFSparseTensorMessage(/*shape=*/{4, 4}, /*num_indptr_buffers=*/1,
580579
/*num_indices_buffers=*/2,
581580
/*axis_order_size=*/3));
582581
ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
583582
}
584583

584+
TEST(TestSparseTensor, RejectNegativeShapeAndNonZeroLength) {
585+
// A negative non_zero_length must be rejected by GetSparseTensorMetadata,
586+
// otherwise the negative size product bypasses the index buffer-size guards.
587+
ASSERT_OK_AND_ASSIGN(
588+
auto message, MakeCSFSparseTensorMessage(/*shape=*/{4, 4}, /*num_indptr_buffers=*/1,
589+
/*num_indices_buffers=*/2,
590+
/*axis_order_size=*/2,
591+
/*non_zero_length=*/-1));
592+
ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
593+
594+
// A negative dimension size must likewise be rejected.
595+
ASSERT_OK_AND_ASSIGN(
596+
message, MakeCSFSparseTensorMessage(/*shape=*/{-1, 4}, /*num_indptr_buffers=*/1,
597+
/*num_indices_buffers=*/2,
598+
/*axis_order_size=*/2));
599+
ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
600+
}
601+
585602
} // namespace test
586603
} // namespace ipc
587604
} // namespace arrow

0 commit comments

Comments
 (0)