decoding malformed Vec<> can cause unrecoverable OOMs irregardless of SizeLimit

[jmesmon]

Example:

extern crate rustc_serialize;
extern crate bincode;

use std::io::Cursor;

#[derive(RustcEncodable)]
struct FakeVec {
    len: u64,
    byte: u8
}

fn main() {

    let x = bincode::encode(&FakeVec { len: 0xffffffffffffffffu64, byte: 1 }, bincode::SizeLimit::Bounded(10)).unwrap();
    let y : Vec<u8> = bincode::decode_from(&mut Cursor::new(&x[..]), bincode::SizeLimit::Bounded(10)).unwrap();
    println!("{:?} {:?}", y, x);
}

Output:

oom-bincode % ./target/debug/oom-bincode
zsh: illegal hardware instruction (core dumped)  ./target/debug/oom-bincode

gdb backtrace:

Core was generated by `./target/debug/oom-bincode'.
Program terminated with signal SIGILL, Illegal instruction.
#0  0x00007f0712686ad0 in oom::hfcd4a1f5b5903c6dX5a ()
warning: Missing auto-load scripts referenced in section .debug_gdb_scripts
of file /home/cody/oom-bincode/target/debug/oom-bincode
Use `info auto-load python-scripts [REGEXP]' to list them.
(gdb) bt
#0  0x00007f0712686ad0 in oom::hfcd4a1f5b5903c6dX5a ()
#1  0x00007f071266c2a4 in oom_bincode::vec::Vec<T>::with_capacity (capacity=18446744073709551615)
    at /home/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/libcollections/vec.rs:223
#2  0x00007f0712670ca4 in fnfn (d=0x7ffd68fec418, len=18446744073709551615)
    at /home/cody/.multirust/toolchains/nightly/cargo/registry/src/github.com-1ecc6299db9ec823/rustc-serialize-0.3.14/src/serialize.rs:458
#3  0x00007f071266f40c in oom_bincode::reader::DecoderReader<'a, R>.Decoder::read_seq<collections::vec::Vec<u8>,closure> (self=0x7ffd68fec418, 
    f={union Result<collections::vec::Vec<u8>, bincode::reader::DecodingError> (struct (&mut bincode::reader::DecoderReader<std::io::cursor::Cursor<&[u8]>>, usize))} 0x7ffd68fec360) at /home/cody/.multirust/toolchains/nightly/cargo/registry/src/github.com-1ecc6299db9ec823/bincode-0.3.0/src/reader.rs:339
#4  0x00007f071266f220 in oom_bincode::serialize::Vec<T>.Decodable::decode<bincode::reader::DecoderReader<std::io::cursor::Cursor<&[u8]>>> (d=0x7ffd68fec418)
    at /home/cody/.multirust/toolchains/nightly/cargo/registry/src/github.com-1ecc6299db9ec823/rustc-serialize-0.3.14/src/serialize.rs:457
#5  0x00007f071266f1bf in oom_bincode::decode_from<std::io::cursor::Cursor<&[u8]>,collections::vec::Vec<u8>> (r=0x7ffd68fec568, size_limit=...)
    at /home/cody/.multirust/toolchains/nightly/cargo/registry/src/github.com-1ecc6299db9ec823/bincode-0.3.0/src/lib.rs:138
#6  0x00007f0712669e3f in oom_bincode::main () at src/main.rs:15
#7  0x00007f07126814a9 in rust_try_inner ()
#8  0x00007f0712681496 in rust_try ()
#9  0x00007f071267ec04 in rt::lang_start::h054bbe55c321aa2c7Gw ()
#10 0x00007f0712672255 in main ()
(gdb) 

[jmesmon]

After starting to use this more extensively, I've come to believe that the check I implimented here is flawed:

If we're decoding a Vec<u8>, the T in read_seq is Vec<u8>, not u8 as I had hoped. This causes the guessed size to be 3 times larger than it actually is (size_of::<Vec<u8>> == 24), which doesn't play nice with stricter bounds.

Also, these checks aren't on the encode size, meaning that I don't see the same bounds failures when encoding (which is a bit annoying).

Anyhow: I'm not sure there is a way for us to grab the size_of the actual type we care about, and these checks won't actually help us if the size of the contained type is larger than the size of the container.

[TyOverby]

If we're decoding a Vec, the T in read_seq is Vec, not u8 as I had hoped

Oh dang. Yeah, that's an issue. I think this could be solved by doing the check inside of the first read_seq_item, and maintaining a stack of lengths to do the checks when we have nested seqs.

Also, these checks aren't on the encode size, meaning that I don't see the same bounds failures when encoding (which is a bit annoying).

This is honestly the worst part for me. I also don't think that it is possible to do because we don't get the T type in the encoder for us to call size_of on.

In the mean time, I'll revert those commits and try to think of a better way to write this.