This is an automated email from the ASF dual-hosted git repository.
pitrou 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 d24791f9df GH-54936: [C++] Ensure reading an invalid map array from
IPC doesn't crash (#54939)
d24791f9df is described below
commit d24791f9df58e4a38e65b27442c1c05d55c71fa6
Author: Antoine Pitrou <[email protected]>
AuthorDate: Thu Oct 29 11:18:29 2026 +0100
GH-54936: [C++] Ensure reading an invalid map array from IPC doesn't crash
(#54939)
### Rationale for this change
When loading a MapArray from IPC, we call
[`MapArray::ValidateChildData`](https://github.com/apache/arrow/blob/b9479757dd5c1914c2b007f11f6679c4fc080c05/cpp/src/arrow/ipc/reader.cc#L456-L459)
to avoid a later crash in `MapArray::SetData`.
Unfortunately `MapArray::ValidateChildData` calls `GetNullCount()` which is
unsafe if the advertised length is too large for the actual validity buffer
size.
Found by OSS-Fuzz in https://issues.oss-fuzz.com/issues/556994537
### What changes are included in this PR?
Check that the `null_count` of the relevant fields is equal to zero, so
that `GetNullCount` does not actually try to read buffer memory.
### Are these changes tested?
Yes, by additional fuzz regression test.
### Are there any user-facing changes?
Just a bugfix.
**This PR contains a "Critical Fix".** An invalid IPC file would lead to
reading memory out of bounds for the purpose of counting validity bits. It is
not obvious how this could be exploited by an attacker except for causing a
denial of service.
### Was AI used for this PR?
**PR code and description written by:**
- [x] Human
- [ ] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
* GitHub Issue: #54936
Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/ipc/reader.cc | 15 +++++++++++++++
testing | 2 +-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc
index 4c10686b19..25a8778330 100644
--- a/cpp/src/arrow/ipc/reader.cc
+++ b/cpp/src/arrow/ipc/reader.cc
@@ -34,6 +34,7 @@
#include <flatbuffers/flatbuffers.h> // IWYU pragma: export
#include "arrow/array.h"
+#include "arrow/array/validate.h"
#include "arrow/buffer.h"
#include "arrow/extension_type.h"
#include "arrow/io/caching.h"
@@ -455,6 +456,20 @@ class ArrayLoader {
Status Visit(const MapType& type) {
RETURN_NOT_OK(LoadList(type));
+ DCHECK_EQ(out_->child_data.size(), 1); // ensured by LoadList()
+ // Ensure that GetNullCount() calls in MapArray::ValidateChildData
+ // don't access invalid memory.
+ // We cannot call array validation as it will fail on non-native endianness
+ // (due to checking the last offset on variable-width data) or missing
+ // dictionaries (they are not yet populated at this point).
+ const ArrayData& child_data = *out_->child_data[0];
+ if (child_data.null_count != 0) {
+ return Status::Invalid("Map array child array should have no nulls");
+ }
+ if (!child_data.child_data.empty() && child_data.child_data[0]->null_count
!= 0) {
+ return Status::Invalid("Map array keys array should have no nulls");
+ }
+ // If we don't do this, MapArray::SetData will abort on invalid data.
return MapArray::ValidateChildData(out_->child_data);
}
diff --git a/testing b/testing
index 9ff285c885..5a26d35785 160000
--- a/testing
+++ b/testing
@@ -1 +1 @@
-Subproject commit 9ff285c88565f0f6abc855918c6a342e70e4909c
+Subproject commit 5a26d35785ff8761e56d79eebff971f6f737e18c
