Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UB in BorshSerialize with padding or uninitialized data #17

Closed
dtolnay opened this issue Mar 13, 2021 · 4 comments · Fixed by #21 or #26
Closed

UB in BorshSerialize with padding or uninitialized data #17

dtolnay opened this issue Mar 13, 2021 · 4 comments · Fixed by #21 or #26
Assignees
Labels
bug Something isn't working

Comments

@dtolnay
Copy link
Contributor

dtolnay commented Mar 13, 2021

The following safe code exhibits UB by reading and dumping out uninitialized memory.

use borsh::BorshSerialize;
use std::io::Result;
use std::mem::MaybeUninit;

#[derive(Copy, Clone)]
struct B(MaybeUninit<u8>);

impl BorshSerialize for B {
    fn serialize<W>(&self, _writer: &mut W) -> Result<()> {
        unreachable!()
    }
    fn is_u8() -> bool {
        true
    }
}

fn main() {
    let x = [B(MaybeUninit::uninit()); 1024];
    println!("{:?}", String::from_utf8_lossy(&x.try_to_vec().unwrap()));
}
$ cargo run --release
"\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}00 103:02
 75240665                  /usr/lib/x86_64-linux-gnu/libgcc_s.so.1\n7f03f10ad000-7f03f10af
000 rw-p 00000000 00:00 0 \n7f03f10af000-7f03f10b0000 r--p 00000000 103:02 75236957       
           /usr/lib/x86_64-linux-gnu/ld-2.31.so\n7f03f10b0000-7f03f10d3000 r-xp 00001000 1
03:02 75236957                  /usr/lib/x86_64-linux-gnu/ld-2.31.so\n7f03f10d3000-7f03f10
db000 r--p 00024000 103:02 75236957                  /usr/lib/x86_64-linux-gnu/ld-2.31.so\
n7f03f10dc000-7f03f10dd000 r--p 0002c000 103:02 75236957                  /usr/lib/x86_64-
linux-gnu/ld-2.31.so\n7f03f10dd000-7f03f10de000 rw-p 0002d000 103:02 75236957             
     /usr/lib/x86_64-linux-gnu/ld-2.31.so\n7f03f10de000-7f03f10df000 rw-p 00000000 00:00 0
 \n7ffeb4418000-7ffeb443a000 rw-p 00000000 00:00 0                          [stack]\n7ffeb
4450000-7ffeb4453000 r--p 00000000 00:00 0                          [vvar]\n7ffeb4453000-7
ffeb4454000 r-xp 00000000 00:00 0                          [vdso]\nffffffffff600000-ffffff
ffff601000 --xp 0000"
@frol
Copy link
Collaborator

frol commented Mar 15, 2021

@evgenykuzyakov Seems to be caused by the same fact of poor communication around safety of is_u8 as in #18 (comment)

@frol frol added the bug Something isn't working label Mar 15, 2021
@evgenykuzyakov
Copy link
Contributor

Yeah, reimplementing is_u8 has to be marked unsafe.

@evgenykuzyakov
Copy link
Contributor

evgenykuzyakov commented Mar 17, 2021

Ideally we would not even export it, but I'm not sure how to implement it. Basically, is_u8 only for u8 type, but Rust is not very flexible.

@dtolnay
Copy link
Contributor Author

dtolnay commented Mar 18, 2021

I think that fix is still wrong. ☹️ I filed #24 to follow up. A function/method marked unsafe exclusively means that it is unsafe to call, not unsafe to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants