Skip to content

fix: Reject lengths whose buffer sizes overflow int64 in minimal validation - #988

Open
pedrobtz wants to merge 1 commit into
apache:mainfrom
pedrobtz:fix-validate-size-overflow
Open

pedrobtz wants to merge 1 commit into
apache:mainfrom
pedrobtz:fix-validate-size-overflow

Conversation

@pedrobtz

Copy link
Copy Markdown

ArrowArrayViewValidateMinimal() checks that offset + length fits an int64_t, then multiplies it by the element size to get each buffer's minimum size. For a fixed-size list it multiplies by the list size to get the child's minimum length. Neither product is checked. A large length overflows the product, which is undefined behaviour. In practice it wraps, and can wrap to a small size that then passes. For an int32 array of length 2^62 + 1, 32 * 4611686018427387905 overflows. Without this change, validation returns NANOARROW_OK with a 16-byte data buffer.

The length is now checked against each buffer's element size in bits before any size is computed, with room for the round-up to whole bytes. It is also checked against the list size before the fixed-size list child length is computed. Lengths that fit behave as before.

The new ArrayViewTestBufferSizeOverflow test validates both cases at the minimal level and expects EINVAL. Without the fix, UBSan reports the overflow and the int32 case returns NANOARROW_OK.

Found by fuzzing the IPC reader in the zuarrow R package, which vendors nanoarrow. An IPC field node can declare any length, and ArrowIpcArrayStreamReader reached this overflow at full validation.

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

…dation

The minimum buffer sizes and the fixed-size list child length were
computed by multiplying offset + length without an overflow check, so a
huge length could wrap around to a small size and pass validation.

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