debris / tiny-keccak

An implementation of Keccak derived functions specified in FIPS-202, SP800-185 and KangarooTwelve
Creative Commons Zero v1.0 Universal
193 stars 49 forks source link

Panic unwinding in Buffer::execute. #55

Open leunga opened 4 months ago

leunga commented 4 months ago

This crate went through a safety review at work and I received the following minor comments from a Rust expert:

#[cfg(target_endian = "big")]
#[inline]
fn execute<F: FnOnce(&mut [u8])>(&mut self, offset: usize, len: usize, f: F) {
    fn swap_endianess(buffer: &mut [u64]) {
        for item in buffer {
            *item = item.swap_bytes();
        }
    }

    let start = offset / 8;
    let end = (offset + len + 7) / 8;
    swap_endianess(&mut self.0[start..end]);
    let buffer: &mut [u8; WORDS * 8] = unsafe { core::mem::transmute(&mut self.0) };  
    f(&mut buffer[offset..][..len]);
    swap_endianess(&mut self.0[start..end]);

"... they do an in-place swap twice instead of just copying the buffer. But I guess the borrow checker makes it impossible for it to be a data race?

I guess if f panics it'll leave this object in an inconsistent state if things unwind. That seems bad, but it's not a safety issue per se. Probably still worth reporting that upstream?"

Just want you to know in case it is an issue.