From cab800615ec1ef7e2bb6f2db19c5d875b55d497b Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 29 Apr 2026 13:45:47 -0400 Subject: [PATCH] Prevent `ArrayData::slice` length overflow (#9813) - None. `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. This updates `ArrayData::slice` to use checked arithmetic when computing the slice end. The panic documentation is updated to describe the overflow case. 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: ```bash cargo test -p arrow-data test_slice_panics_on_offset_length_overflow --release ``` Invalid `ArrayData::slice` arguments where `offset + length` overflows now panic consistently across build modes. There are no API changes. --- .../src/array/fixed_size_list_array.rs | 2 +- arrow-array/src/array/struct_array.rs | 2 +- arrow-data/src/data.rs | 20 +++++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/arrow-array/src/array/fixed_size_list_array.rs b/arrow-array/src/array/fixed_size_list_array.rs index 4a338591e5aa..637767996dbc 100644 --- a/arrow-array/src/array/fixed_size_list_array.rs +++ b/arrow-array/src/array/fixed_size_list_array.rs @@ -539,7 +539,7 @@ mod tests { } #[test] - #[should_panic(expected = "assertion failed: (offset + length) <= self.len()")] + #[should_panic(expected = "assertion failed: end <= self.len()")] // Different error messages, so skip for now // https://github.com/apache/arrow-rs/issues/1545 #[cfg(not(feature = "force_validate"))] diff --git a/arrow-array/src/array/struct_array.rs b/arrow-array/src/array/struct_array.rs index fbc34ef0c85b..7e35eb6450b2 100644 --- a/arrow-array/src/array/struct_array.rs +++ b/arrow-array/src/array/struct_array.rs @@ -642,7 +642,7 @@ mod tests { } #[test] - #[should_panic(expected = "assertion failed: (offset + length) <= self.len()")] + #[should_panic(expected = "assertion failed: end <= self.len()")] fn test_struct_array_from_data_with_offset_and_length_error() { let int_arr = Int32Array::from(vec![1, 2, 3, 4, 5]); let int_field = Field::new("x", DataType::Int32, false); diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs index fca19bc3aafe..302ee12f3fa5 100644 --- a/arrow-data/src/data.rs +++ b/arrow-data/src/data.rs @@ -547,9 +547,12 @@ impl ArrayData { /// /// # Panics /// - /// Panics if `offset + length > self.len()`. + /// Panics if `offset + length` overflows or is greater than `self.len()`. pub fn slice(&self, offset: usize, length: usize) -> ArrayData { - assert!((offset + length) <= self.len()); + let end = offset + .checked_add(length) + .expect("offset + length overflow"); + assert!(end <= self.len()); if let DataType::Struct(_) = self.data_type() { // Slice into children @@ -2258,6 +2261,19 @@ mod tests { assert_eq!(data.null_count() - 1, new_data.null_count()); } + #[test] + #[should_panic(expected = "offset + length overflow")] + fn test_slice_panics_on_offset_length_overflow() { + let data = ArrayData::builder(DataType::Int32) + .len(4) + .add_buffer(make_i32_buffer(4)) + .build() + .unwrap(); + let sliced = data.slice(1, 3); + + sliced.slice(1, usize::MAX); + } + #[test] fn test_equality() { let int_data = ArrayData::builder(DataType::Int32)