spacejam/sled

`IoBuf::get_mut_range` unsoundly extends arbitrary lifetime to 'static

vikramnitin9 opened this issue · 2 comments

This method takes &self and returns a 'static bounded reference.

pub(crate) fn get_mut_range(
&self,
at: usize,
len: usize,
) -> &'static mut [u8] {
let buf_ptr = self.buf.get();
unsafe {
assert!((*buf_ptr).1 >= at + len);
std::slice::from_raw_parts_mut(
(*buf_ptr).0.add(self.base + at),
len,
)
}
}

This can cause a use-after-free. The following example fails Miri:

use std::{
    alloc::{alloc, dealloc, Layout},
    sync::Arc,
    cell::UnsafeCell
};

struct AlignedBuf(*mut u8, usize);

impl AlignedBuf {
    fn new(len: usize) -> AlignedBuf {
        let layout = Layout::from_size_align(len, 8192).unwrap();
        let ptr = unsafe { alloc(layout) };

        assert!(!ptr.is_null(), "failed to allocate critical IO buffer");

        AlignedBuf(ptr, len)
    }
}

impl Drop for AlignedBuf {
    fn drop(&mut self) {
        let layout = Layout::from_size_align(self.1, 8192).unwrap();
        unsafe {
            dealloc(self.0, layout);
        }
    }
}

pub(crate) struct IoBuf {
    buf: Arc<UnsafeCell<AlignedBuf>>,
    base: usize,
}

impl IoBuf {
    pub(crate) fn get_mut_range(
        &self,
        at: usize,
        len: usize,
    ) -> &'static mut [u8] {
        let buf_ptr = self.buf.get();

        unsafe {
            assert!((*buf_ptr).1 >= at + len);
            std::slice::from_raw_parts_mut(
                (*buf_ptr).0.add(self.base + at),
                len,
            )
        }
    }

}

fn main() {
    let mut mut_range;
    {
        let iobuf = IoBuf {
            buf: Arc::new(UnsafeCell::new(AlignedBuf::new(16))),
            base: 0
        };

        mut_range = iobuf.get_mut_range(4, 2);
        mut_range[0] = 8;
        mut_range[1] = 9;
    }
    // Use after free
    println!("hello {:?} :)", mut_range);
}

A tangential issue about this same function had been discussed in #1044, but I think that the function as it stands is still unsafe and should be marked as such.

i have a plan to rewrite alignedBuf to mostly safe code in #1487
it would completely solve this issue. I'll try to make a PR soon.

fixed in #1488