GH-49059: [C++] Fix issues found by OSS-Fuzz in IPC reader

Author: pitrou

cpp/src/arrow/ipc/reader.cc

@@ -245,7 +245,7 @@ class ArrayLoader {
   }
 
   Status GetBuffer(int buffer_index, std::shared_ptr<Buffer>* out) {
-    auto buffers = metadata_->buffers();
+    auto* buffers = metadata_->buffers();
     CHECK_FLATBUFFERS_NOT_NULL(buffers, "RecordBatch.buffers");
     if (buffer_index >= static_cast<int>(buffers->size())) {
       return Status::IOError("buffer_index out of range.");
@@ -262,7 +262,9 @@ class ArrayLoader {
 
   Result<int64_t> GetVariadicCount(int i) {
     auto* variadic_counts = metadata_->variadicBufferCounts();
+    auto* buffers = metadata_->buffers();
     CHECK_FLATBUFFERS_NOT_NULL(variadic_counts, "RecordBatch.variadicBufferCounts");
+    CHECK_FLATBUFFERS_NOT_NULL(buffers, "RecordBatch.buffers");
     if (i >= static_cast<int>(variadic_counts->size())) {
       return Status::IOError("variadic_count_index out of range.");
     }
@@ -272,8 +274,7 @@ class ArrayLoader {
     }
     // Detect an excessive variadic buffer count to avoid potential memory blowup
     // (GH-48900).
-    const auto max_buffer_count =
-        static_cast<int64_t>(metadata_->buffers()->size()) - buffer_index_;
+    const auto max_buffer_count = static_cast<int64_t>(buffers->size()) - buffer_index_;
     if (count > max_buffer_count) {
       return Status::IOError("variadic buffer count exceeds available number of buffers");
     }

cpp/src/arrow/record_batch.cc

@@ -266,10 +266,13 @@ Result<std::shared_ptr<RecordBatch>> RecordBatch::FromStructArray(
 namespace {
 
 Status ValidateColumnLength(const RecordBatch& batch, int i) {
-  const auto& array = *batch.column(i);
-  if (ARROW_PREDICT_FALSE(array.length() != batch.num_rows())) {
+  // This function is part of the validation code path and should
+  // be robust against invalid data, but `column()` would call MakeArray()
+  // that can abort on invalid data.
+  const auto& array = *batch.column_data(i);
+  if (ARROW_PREDICT_FALSE(array.length != batch.num_rows())) {
     return Status::Invalid("Number of rows in column ", i,
-                           " did not match batch: ", array.length(), " vs ",
+                           " did not match batch: ", array.length, " vs ",
                            batch.num_rows());
   }
   return Status::OK();
@@ -455,11 +458,12 @@ namespace {
 Status ValidateBatch(const RecordBatch& batch, bool full_validation) {
   for (int i = 0; i < batch.num_columns(); ++i) {
     RETURN_NOT_OK(ValidateColumnLength(batch, i));
-    const auto& array = *batch.column(i);
+    // See ValidateColumnLength about avoiding a ArrayData -> Array conversion
+    const auto& array = *batch.column_data(i);
     const auto& schema_type = batch.schema()->field(i)->type();
-    if (!array.type()->Equals(schema_type)) {
+    if (!array.type->Equals(schema_type)) {
       return Status::Invalid("Column ", i,
-                             " type not match schema: ", array.type()->ToString(), " vs ",
+                             " type not match schema: ", array.type->ToString(), " vs ",
                              schema_type->ToString());
     }
     const auto st = full_validation ? internal::ValidateArrayFull(array)

cpp/src/arrow/record_batch_test.cc

@@ -318,7 +318,6 @@ TEST_F(TestRecordBatch, Validate) {
   auto a3 = gen.ArrayOf(int16(), 5);
 
   auto b1 = RecordBatch::Make(schema, length, {a0, a1, a2});
-
   ASSERT_OK(b1->ValidateFull());
 
   // Length mismatch
@@ -328,6 +327,21 @@ TEST_F(TestRecordBatch, Validate) {
   // Type mismatch
   auto b3 = RecordBatch::Make(schema, length, {a0, a1, a0});
   ASSERT_RAISES(Invalid, b3->ValidateFull());
+
+  // Invalid column data (nulls in map key array) that would abort on MakeArray
+  auto map_field = field("f", map(utf8(), int32()));
+  schema = ::arrow::schema({map_field});
+  auto map_key_data = ArrayFromJSON(utf8(), "[null]")->data();
+  auto map_item_data = ArrayFromJSON(int32(), "[null]")->data();
+  auto map_data = ArrayData::Make(map_field->type(), /*length=*/1, /*buffers=*/{nullptr},
+                                  /*child_data=*/{map_key_data, map_item_data});
+
+  auto b4 = RecordBatch::Make(schema, /*num_rows=*/map_data->length, {map_data});
+  ASSERT_RAISES(Invalid, b4->ValidateFull());
+
+  // Length mismatch with a column data that would also fail on MakeArray
+  auto b5 = RecordBatch::Make(schema, /*num_rows=*/1 + map_data->length, {map_data});
+  ASSERT_RAISES(Invalid, b5->Validate());
 }
 
 TEST_F(TestRecordBatch, Slice) {