This is an automated email from the ASF dual-hosted git repository.
rok pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new e92d5d7977 GH-50161: [C++][IPC] Validate CSF sparse index buffer
counts (#50070)
e92d5d7977 is described below
commit e92d5d7977f0a753ec9557f204343a33cb1cda93
Author: metsw24-max <[email protected]>
AuthorDate: Tue Jun 30 18:49:11 2026 +0530
GH-50161: [C++][IPC] Validate CSF sparse index buffer counts (#50070)
### Rationale for this change
`ReadSparseCSFIndex` in cpp/src/arrow/ipc/reader.cc sizes
`indptr_data`/`indices_data` from the tensor shape (`ndim - 1`/`ndim`) but
fills them by looping over the flatbuffer-supplied buffer counts, which are
never checked against `ndim`. A crafted CSF sparse tensor message with
mismatched counts writes past the end of those vectors, and `ndim == 0` builds
a `SIZE_MAX`-sized vector. The payload path is already guarded by
`CheckSparseTensorBodyBufferCount`; the file/stream path was not [...]
### What changes are included in this PR?
Callee-side count validation in both functions: reject CSF indices whose
`indptrBuffers`/`indicesBuffers` counts don't match `ndim - 1`/`ndim` (and
`ndim < 1`), and whose `axisOrder`/`indicesBuffers` lengths differ, with
`Status::Invalid`.
### Are these changes tested?
Covered by the existing sparse tensor IPC round-trip tests; the rejected
inputs are only producible from hand-crafted flatbuffers.
### Are there any user-facing changes?
No, only invalid inputs are rejected.
**This PR contains a "Critical Fix".** A crafted IPC sparse tensor message
could trigger a heap out-of-bounds write (`shared_ptr<Buffer>` constructions
past the vector end) or an out-of-bounds flatbuffer read from the public
`ReadSparseTensor` API.
* GitHub Issue: #50161
Lead-authored-by: metsw24-max <[email protected]>
Co-authored-by: Sayed Kaif <[email protected]>
Signed-off-by: Rok Mihevc <[email protected]>
---
cpp/src/arrow/ipc/metadata_internal.cc | 15 +++++--
cpp/src/arrow/ipc/reader.cc | 7 +++
cpp/src/arrow/ipc/tensor_test.cc | 82 ++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 3 deletions(-)
diff --git a/cpp/src/arrow/ipc/metadata_internal.cc
b/cpp/src/arrow/ipc/metadata_internal.cc
index a1308411e2..eef5cc221f 100644
--- a/cpp/src/arrow/ipc/metadata_internal.cc
+++ b/cpp/src/arrow/ipc/metadata_internal.cc
@@ -1522,10 +1522,19 @@ Status GetSparseCSFIndexMetadata(const
flatbuf::SparseTensorIndexCSF* sparse_ind
RETURN_NOT_OK(IntFromFlatbuffer(sparse_index->indptrType(), indptr_type));
RETURN_NOT_OK(IntFromFlatbuffer(sparse_index->indicesType(), indices_type));
- const int ndim = static_cast<int>(sparse_index->axisOrder()->size());
+ auto* fb_axis_order = sparse_index->axisOrder();
+ auto* fb_indices_buffers = sparse_index->indicesBuffers();
+ if (fb_axis_order == nullptr || fb_indices_buffers == nullptr ||
+ fb_axis_order->size() != fb_indices_buffers->size()) {
+ return Status::Invalid(
+ "Inconsistent CSF sparse index: axisOrder and indicesBuffers have
different "
+ "lengths");
+ }
+
+ const int ndim = static_cast<int>(fb_axis_order->size());
for (int i = 0; i < ndim; ++i) {
- axis_order->push_back(sparse_index->axisOrder()->Get(i));
- indices_size->push_back(sparse_index->indicesBuffers()->Get(i)->length());
+ axis_order->push_back(fb_axis_order->Get(i));
+ indices_size->push_back(fb_indices_buffers->Get(i)->length());
}
return Status::OK();
diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc
index a7d24fa86c..376d0fab4b 100644
--- a/cpp/src/arrow/ipc/reader.cc
+++ b/cpp/src/arrow/ipc/reader.cc
@@ -2413,6 +2413,13 @@ Result<std::shared_ptr<SparseIndex>> ReadSparseCSFIndex(
const auto ndim = static_cast<int64_t>(shape.size());
auto* indptr_buffers = sparse_index->indptrBuffers();
auto* indices_buffers = sparse_index->indicesBuffers();
+ if (ndim < 2 || indptr_buffers == nullptr || indices_buffers == nullptr ||
+ static_cast<int64_t>(indptr_buffers->size()) != ndim - 1 ||
+ static_cast<int64_t>(indices_buffers->size()) != ndim) {
+ return Status::Invalid(
+ "Inconsistent CSF sparse index: a CSF tensor must have at least 2
dimensions "
+ "with indptr and indices buffer counts of ndim - 1 and ndim");
+ }
std::vector<std::shared_ptr<Buffer>> indptr_data(ndim - 1);
std::vector<std::shared_ptr<Buffer>> indices_data(ndim);
diff --git a/cpp/src/arrow/ipc/tensor_test.cc b/cpp/src/arrow/ipc/tensor_test.cc
index 2706ea19d5..4890dfc992 100644
--- a/cpp/src/arrow/ipc/tensor_test.cc
+++ b/cpp/src/arrow/ipc/tensor_test.cc
@@ -24,9 +24,14 @@
#include <gtest/gtest.h>
+#include <flatbuffers/flatbuffers.h>
+
+#include "arrow/buffer.h"
#include "arrow/io/file.h"
#include "arrow/io/memory.h"
#include "arrow/io/test_common.h"
+#include "arrow/ipc/message.h"
+#include "arrow/ipc/metadata_internal.h"
#include "arrow/ipc/reader.h"
#include "arrow/ipc/test_common.h"
#include "arrow/ipc/writer.h"
@@ -500,6 +505,83 @@ INSTANTIATE_TYPED_TEST_SUITE_P(TestInt32,
TestSparseTensorRoundTrip, Int32Type);
INSTANTIATE_TYPED_TEST_SUITE_P(TestUInt32, TestSparseTensorRoundTrip,
UInt32Type);
INSTANTIATE_TYPED_TEST_SUITE_P(TestInt64, TestSparseTensorRoundTrip,
Int64Type);
+namespace {
+
+// Build a SparseTensor IPC message carrying a CSF index with caller-controlled
+// buffer counts. This lets us exercise the reader's validation directly, since
+// such inconsistent indices can only be produced by hand-crafted flatbuffers.
+Result<std::shared_ptr<Message>> MakeCSFSparseTensorMessage(int num_dims,
+ int
num_indptr_buffers,
+ int
num_indices_buffers,
+ int
axis_order_size) {
+ flatbuffers::FlatBufferBuilder fbb;
+
+ auto value_type = flatbuf::CreateInt(fbb, 64, /*is_signed=*/true);
+
+ std::vector<flatbuffers::Offset<flatbuf::TensorDim>> dims;
+ for (int i = 0; i < num_dims; ++i) {
+ dims.push_back(flatbuf::CreateTensorDim(fbb, /*size=*/4, /*name=*/0));
+ }
+ auto fb_shape = fbb.CreateVector(dims);
+
+ auto indptr_type = flatbuf::CreateInt(fbb, 64, /*is_signed=*/false);
+ auto indices_type = flatbuf::CreateInt(fbb, 64, /*is_signed=*/false);
+
+ std::vector<flatbuf::Buffer> indptr(num_indptr_buffers, flatbuf::Buffer(0,
0));
+ std::vector<flatbuf::Buffer> indices(num_indices_buffers, flatbuf::Buffer(0,
0));
+ auto fb_indptr = fbb.CreateVectorOfStructs(indptr);
+ auto fb_indices = fbb.CreateVectorOfStructs(indices);
+
+ std::vector<int32_t> axis_order(axis_order_size, 0);
+ auto fb_axis_order = fbb.CreateVector(axis_order);
+
+ auto csf = flatbuf::CreateSparseTensorIndexCSF(fbb, indptr_type, fb_indptr,
+ indices_type, fb_indices,
fb_axis_order);
+
+ flatbuf::Buffer data(0, 0);
+ auto sparse_tensor = flatbuf::CreateSparseTensor(
+ fbb, flatbuf::Type_Int, value_type.Union(), fb_shape,
/*non_zero_length=*/0,
+ flatbuf::SparseTensorIndex::SparseTensorIndex_SparseTensorIndexCSF,
csf.Union(),
+ &data);
+
+ fbb.Finish(flatbuf::CreateMessage(fbb, internal::kCurrentMetadataVersion,
+
flatbuf::MessageHeader::MessageHeader_SparseTensor,
+ sparse_tensor.Union(),
+ /*bodyLength=*/0));
+
+ ARROW_ASSIGN_OR_RAISE(auto metadata, internal::WriteFlatbufferBuilder(fbb));
+ auto body = Buffer::FromString(std::string(8, '\0'));
+ ARROW_ASSIGN_OR_RAISE(auto message, Message::Open(metadata, body));
+ return std::shared_ptr<Message>(std::move(message));
+}
+
+} // namespace
+
+TEST(TestSparseCSFIndex, RejectInconsistentBufferCounts) {
+ // ndim == 1 is not a valid CSF index (it has no indptr buffers), and used to
+ // reach SparseCSFIndex's constructor with an empty indptr vector.
+ ASSERT_OK_AND_ASSIGN(
+ auto message, MakeCSFSparseTensorMessage(/*num_dims=*/1,
/*num_indptr_buffers=*/0,
+ /*num_indices_buffers=*/1,
+ /*axis_order_size=*/1));
+ ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
+
+ // Too many indices buffers for the declared number of dimensions, which used
+ // to write past the end of the fixed-size index vectors.
+ ASSERT_OK_AND_ASSIGN(
+ message, MakeCSFSparseTensorMessage(/*num_dims=*/2,
/*num_indptr_buffers=*/1,
+ /*num_indices_buffers=*/3,
+ /*axis_order_size=*/2));
+ ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
+
+ // axisOrder and indicesBuffers lengths disagree (out-of-bounds read).
+ ASSERT_OK_AND_ASSIGN(
+ message, MakeCSFSparseTensorMessage(/*num_dims=*/2,
/*num_indptr_buffers=*/1,
+ /*num_indices_buffers=*/2,
+ /*axis_order_size=*/3));
+ ASSERT_RAISES(Invalid, ReadSparseTensor(*message));
+}
+
} // namespace test
} // namespace ipc
} // namespace arrow