Skip to content

fix: Don't read offsets from a too-short offsets buffer in default validation - #987

Open
pedrobtz wants to merge 1 commit into
apache:mainfrom
pedrobtz:fix-validate-short-offsets
Open

pedrobtz wants to merge 1 commit into
apache:mainfrom
pedrobtz:fix-validate-short-offsets

Conversation

@pedrobtz

Copy link
Copy Markdown

ArrowArrayViewValidateMinimal() deliberately allows an empty offsets buffer for a zero-length array. Its minimum size is (offset_plus_length != 0) * element_size * (offset_plus_length + 1), which is 0 for a zero-length array. So a buffer of 1 to 3 bytes (or 1 to 7 for the large types) passes. ArrowArrayViewValidateDefault() then reads offsets whenever size_bytes != 0, which reads a whole int32_t or int64_t past the end of the buffer.

Default validation now reads the first and last offsets only when the buffer holds offset_plus_length + 1 of them. This applies to string and binary, large string and binary, list and map, and large list. A buffer that is long enough behaves as before. A too-short buffer for a zero-length array is treated like an empty one.

The new ArrayViewTestShortOffsetsBuffer test validates a zero-length string, large string, list and large list. Each has an offsets buffer one byte short of a single offset, in a heap allocation of exactly that size. Without the fix, ASan reports a heap-buffer-overflow. With it, all four validate.

The input was found by fuzzing the IPC reader in the zuarrow R package, which vendors nanoarrow. An IPC message can declare a 2-byte offsets buffer for a zero-length string column.

ctest passes in a plain Debug build and with ASan and UBSan (410 tests each). The changed files are formatted with the pinned clang-format 16.0.6.

🤖 Generated with Claude Code

…lidation

Minimal validation allows a zero-length array to have an offsets buffer
shorter than one offset. Default validation then read a full offset from
it. Read offsets only when the buffer holds them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant