Skip to content

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

Open
pedrobtz wants to merge 2 commits into
apache:mainfrom
pedrobtz:fix-validate-short-offsets
Open

pedrobtz wants to merge 2 commits 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>
@codecov-commenter

codecov-commenter commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.52%. Comparing base (08d23f6) to head (9a8bc11).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #987   +/-   ##
=======================================
  Coverage   78.52%   78.52%           
=======================================
  Files         106      106           
  Lines       17195    17199    +4     
  Branches     2064     2064           
=======================================
+ Hits        13502    13506    +4     
  Misses       2431     2431           
  Partials     1262     1262           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants