[ARROW-10562] [Rust] Potential UB on unsafe code

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 3.0.0
• Component/s: Rust
• External issue URL: https://github.com/apache/arrow/issues/26527
• Assignee: Jorge Leitão
• Reporter: Jorge Leitão
• Created: 12/Nov/20 08:00
• Updated: 11/Jan/23 08:14
• Resolved: 14/Nov/20 07:28

Description

The function set_bits_raw may be (unsafely) accessing memory out of bounds.

Specifically, if I re-write it in a safe manner, it panics (instead of UB)

pub fn set_bits_raw(data: &mut [u8], start: usize, end: usize) {
    let start_byte = start >> 3;
    let end_byte = end >> 3;

    let start_offset = (start & 7) as u8;
    let end_offset = (end & 7) as u8;

    // All set apart from lowest `start_offset` bits
    let start_mask = !((1 << start_offset) - 1);
    // All clear apart from lowest `end_offset` bits
    let end_mask = (1 << end_offset) - 1;

    if start_byte == end_byte {
        data[start_byte] |= start_mask & end_mask;
    } else {
        data[start_byte] |= start_mask;
        for i in (start_byte + 1)..end_byte {
            data[i] = 0xFF;
        }
        data[end_byte] |= end_mask;
    }
}

A test that panics (and that represents an operation that we peform in `BufferBuilder` would be)

    #[test]
    fn test_set_bits_raw_from_start() {
        let mut buf = vec![0; 1];
        unsafe {set_bits_raw(buf.as_mut(), 0, 0)};
        // 00000000
        assert_eq!(buf[0], 0);
        unsafe {set_bits_raw(buf.as_mut(), 0, 1)};
        // 00000001
        assert_eq!(buf[0], 1);
        unsafe {set_bits_raw(buf.as_mut(), 0, 2)};
        // 00000001
        assert_eq!(buf[0], 3);
        unsafe {set_bits_raw(buf.as_mut(), 0, 7)};
        // 01111111
        assert_eq!(buf[0], 127);
        unsafe {set_bits_raw(buf.as_mut(), 0, 8)};
        // 11111111
        assert_eq!(buf[0], 255);
    }

I think that it is related to the fact that the documentation states that the bounds are non-inclusive, but the `end` seems to be inclusive.