Skip to content

transmute_state is technically undefined behavior #15

@cmlsharp

Description

@cmlsharp

merlin/src/strobe.rs

Lines 18 to 20 in a94892f

fn transmute_state(st: &mut AlignedKeccakState) -> &mut [u64; 25] {
unsafe { &mut *(st as *mut AlignedKeccakState as *mut [u64; 25]) }
}

This code makes the assumption that AlignedKeccakState has the same layout as its singular field [u8; 200], but this is not guaranteed by #[repr(align(8))]. I believe the compiler would technically be free to to, say, add 8 bytes of padding to the beginning of AlignedKeccakState. Either changing the repr attribute to #[repr(C, align(8))] or changing the definition of transmute_state to e.g. unsafe { &mut *(&raw mut st.0 as *mut [u64; 25]) } would do it (for little endian targets anyway). (Edit: actually I think you just need repr(C, align(8)). Without repr C there’s no guarantee that the first member of an 8 byte aligned struct is 8 byte aligned)

Alternatively, I think a union makes things a bit more explicit here:

#[repr(C)]
#[derive(Copy, Clone)]
union KeccakState {
    bytes: [u8; Self::BYTE_LEN],
    words: [u64; Self::WORD_LEN],
}

impl KeccakState {
    const WORD_LEN: usize = 25;
    const BYTE_LEN: usize = Self::WORD_LEN * 8;

    #[inline]
    fn as_mut_bytes(&mut self) -> &mut [u8; Self::BYTE_LEN] {
        // # SAFETY:
        // repr(C) guarantees bytes and words start at the same address
        // and both are the same size, so this is always safe to access
        unsafe { &mut self.bytes }
    }

    #[inline]
    fn as_mut_words(&mut self) -> &mut [u64; Self::WORD_LEN] {
        // # SAFETY:
        // repr(C) guarantees bytes and words start at the same address
        // and both are the same size, so this is always safe to access
        unsafe { &mut self.words }
    }

    #[inline]
    fn permute(&mut self) {
        self.normalize_endianness();
        keccak::f1600(self.as_mut_words());
        self.normalize_endianness();
    }

    /// on big-endian platforms, swap byte order before calling keccak
    /// to ensure e.g. a big-endian prover and little endian verifier agree
    /// on transcript
    #[inline]
    fn normalize_endianness(&mut self) {
        if cfg!(target_endian = "big") {
            self.as_mut_words()
                .iter_mut()
                .for_each(|word| *word = word.swap_bytes());
        }
    }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions