Skip to content

Prevent ArrayData::slice length overflow#9813

Open
alamb wants to merge 3 commits intoapache:mainfrom
alamb:codex/arraydata-slice-overflow
Open

Prevent ArrayData::slice length overflow#9813
alamb wants to merge 3 commits intoapache:mainfrom
alamb:codex/arraydata-slice-overflow

Conversation

@alamb
Copy link
Copy Markdown
Contributor

@alamb alamb commented Apr 25, 2026

Which issue does this PR close?

  • None.

Rationale for this change

ArrayData::slice checked bounds using unchecked usize addition. In optimized builds, very large length values could wrap offset + length, allowing invalid slice arguments to create ArrayData with inconsistent length and offset metadata instead of panicking.

What changes are included in this PR?

This updates ArrayData::slice to use checked arithmetic when computing the slice end.

The panic documentation is updated to describe the overflow case.

Are these changes tested?

Yes. This adds a regression test covering a slice-of-slice call where offset + length overflows. The new regression was also verified in release mode.

Validated with:

cargo test -p arrow-data test_slice_panics_on_offset_length_overflow --release

Are there any user-facing changes?

Invalid ArrayData::slice arguments where offset + length overflows now panic consistently across build modes. There are no API changes.

@github-actions github-actions Bot added the arrow Changes to the arrow crate label Apr 25, 2026
@alamb alamb changed the title [arrow-data]: prevent ArrayData::slice length overflow Prevent ArrayData::slice length overflow Apr 25, 2026
Comment thread arrow-data/src/data.rs
assert_eq!(data.null_count() - 1, new_data.null_count());
}

#[test]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how the test fails without the code change

It seems like maybe we should start running some targeted tests in release builds too to really ensure these cases are covered 🤔

andrewlamb@Andrews-MacBook-Pro-3:~/Software/arrow-rs$ cargo test -p arrow-data test_slice_panics_on_offset_length_overflow
   Compiling arrow-data v58.1.0 (/Users/andrewlamb/Software/arrow-rs/arrow-data)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.45s
     Running unittests src/lib.rs (target/debug/deps/arrow_data-c6408ac97be22d82)

running 1 test
test data::tests::test_slice_panics_on_offset_length_overflow - should panic ... FAILED

failures:

---- data::tests::test_slice_panics_on_offset_length_overflow stdout ----

thread 'data::tests::test_slice_panics_on_offset_length_overflow' (46323764) panicked at arrow-data/src/data.rs:581:17:
attempt to add with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: panic did not contain expected string
      panic message: "attempt to add with overflow"
 expected substring: "offset + length overflow"

failures:
    data::tests::test_slice_panics_on_offset_length_overflow

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 27 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p arrow-data --lib`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alamb alamb marked this pull request as ready for review April 25, 2026 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant