(arrow-rs) branch main updated: fix: Used `checked_add` for bounds checks to avoid UB (#9568)

[alamb]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/main by this push:
     new 42ab0bcef7 fix: Used `checked_add` for bounds checks to avoid UB 
(#9568)
42ab0bcef7 is described below

commit 42ab0bcef7c2257772dfb7de77b04051350e18cb
Author: Ed Seidl <etse...@users.noreply.github.com>
AuthorDate: Thu Mar 19 10:39:35 2026 -0700

    fix: Used `checked_add` for bounds checks to avoid UB (#9568)
    
    # Which issue does this PR close?
    
    - Closes #9543.
    
    # Rationale for this change
    
    See issue, but it is possible to construct arguments to
    `arrow_buffer::bit_util::bit_mask::set_bits` that overflow the bounds
    checking protecting unsafe code.
    
    # What changes are included in this PR?
    Use `checked_add` when doing the bounds checking and panic when an
    overflow occurs.
    
    # Are these changes tested?
    
    Yes
    
    # Are there any user-facing changes?
    
    No
---
 arrow-buffer/src/util/bit_mask.rs | 50 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/arrow-buffer/src/util/bit_mask.rs 
b/arrow-buffer/src/util/bit_mask.rs
index a8ae1a7654..e3897e6754 100644
--- a/arrow-buffer/src/util/bit_mask.rs
+++ b/arrow-buffer/src/util/bit_mask.rs
@@ -32,8 +32,18 @@ pub fn set_bits(
     offset_read: usize,
     len: usize,
 ) -> usize {
-    assert!(offset_write + len <= write_data.len() * 8);
-    assert!(offset_read + len <= data.len() * 8);
+    assert!(
+        offset_write
+            .checked_add(len)
+            .expect("operation will overflow write buffer")
+            <= write_data.len() * 8
+    );
+    assert!(
+        offset_read
+            .checked_add(len)
+            .expect("operation will overflow read buffer")
+            <= data.len() * 8
+    );
     let mut null_count = 0;
     let mut acc = 0;
     while len > acc {
@@ -427,4 +437,40 @@ mod tests {
         assert_eq!(len_set, 1);
         assert_eq!(write_data, &[0b00000010]);
     }
+
+    #[test]
+    #[should_panic(expected = "operation will overflow read buffer")]
+    fn test_overflow_read_buffer_bounds() {
+        // Tiny buffers so any huge computed index is out-of-bounds.
+        let data = [0u8; 1];
+        let mut write_data = [0u8; 1];
+
+        // Choose values so (offset_read + len) wraps to a small number in 
release builds.
+        // offset_read = usize::MAX - 7, len = 8 => wraps to 0.
+        // This can bypass `assert!(offset_read + len <= data.len() * 8)`.
+        let offset_write: usize = 0;
+        let offset_read: usize = usize::MAX - 7;
+        let len: usize = 8;
+
+        // should panic on bounds check overflow
+        let _nulls = set_bits(&mut write_data, &data, offset_write, 
offset_read, len);
+    }
+
+    #[test]
+    #[should_panic(expected = "operation will overflow write buffer")]
+    fn test_overflow_write_buffer_bounds() {
+        // Tiny buffers so any huge computed index is out-of-bounds.
+        let data = [0u8; 1];
+        let mut write_data = [0u8; 1];
+
+        // Choose values so (offset_write + len) wraps to a small number in 
release builds.
+        // offset_write = usize::MAX - 7, len = 8 => wraps to 0.
+        // This can bypass `assert!(offset_write + len <= write_data.len() * 
8)`.
+        let offset_write: usize = usize::MAX - 7;
+        let offset_read: usize = 0;
+        let len: usize = 8;
+
+        // should panic on bounds check overflow
+        let _nulls = set_bits(&mut write_data, &data, offset_write, 
offset_read, len);
+    }
 }