copy_into_slice and related methods can be used to read/write out of bounds

[fpoli]

By using the unstable features of this crate, it's possible to cause a segmentation fault without writing any unsafe code. I wrote an example below. The reason is that the runtime checks of copy_into_slice and similar methods optimistically assume that the implementation of Deref and DerefMut always returns the same reference. To fix this, I think that there should be at most one call to self.reference.{deref|deref_mut}() in each of those methods.

src/main.rs:

use std::cell::Cell;
use std::ops::Deref;
use volatile::Volatile;

#[repr(C)]
struct Pair {
    counter: Cell<u32>,
    long: [u8; 1000],
    short: [u8; 1],
}

impl Pair {
    fn new() -> Pair {
        Pair {
            counter: Cell::new(0),
            long: [0; 1000],
            short: [0; 1],
        }
    }
}

impl Deref for Pair {
    type Target = [u8];

    fn deref(&self) -> &[u8] {
        let c = self.counter.get();
        self.counter.set(c + 1);

        if c % 3 == 0 {
            &self.short
        } else {
            &self.long
        }
    }
}

fn main() {
    let volatile = Volatile::new(Pair::new());
    let mut dst: [u8; 1] = [1];
    // This will try to write `long` into `dst`, going out of bounds
    volatile.copy_into_slice(&mut dst);
}

Cargo.toml:

[package]
name = "volatile_bug"
version = "0.1.0"
edition = "2021"

[dependencies]
volatile = { version = "0.4.5", features = ["unstable"] }

Output of cargo run:

Segmentation fault (core dumped)

[phil-opp]

Thanks a lot for reporting! Calling deref/deref_mut only once seems like the correct solution.

Unfortunately, I won't have time to look into this right now, but I put it on my list and try to fix it soon.