bincode 2 overallocates on untrusted inputs

[udoprog]

I discovered this when fuzzing bincode using musli tests:

RUST_BACKTRACE=1 cargo run --features bincode-derive -p tests --bin fuzz -- --random

Basically what this harness does is feed the decoder random bytes.

What you see is something like this:

Panic due to overallocating

thread 'main' panicked at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/impl_alloc.rs:269:27:
capacity overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/panicking.rs:692:5
   1: core::panicking::panic_fmt
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/core/src/panicking.rs:75:14
   2: alloc::raw_vec::capacity_overflow
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/alloc/src/raw_vec.rs:25:5
   3: alloc::raw_vec::handle_error
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/alloc/src/raw_vec.rs:791:29
   4: alloc::raw_vec::RawVecInner<A>::with_capacity_zeroed_in
             at /home/udoprog/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/raw_vec.rs:447:25
   5: alloc::raw_vec::RawVec<T,A>::with_capacity_zeroed_in
             at /home/udoprog/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/raw_vec.rs:214:20
   6: <u8 as alloc::vec::spec_from_elem::SpecFromElem>::from_elem
             at /home/udoprog/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/spec_from_elem.rs:55:31
   7: alloc::vec::from_elem
             at /home/udoprog/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:3190:5
   8: bincode::features::impl_alloc::<impl bincode::de::Decode<Context> for alloc::vec::Vec<T>>::decode
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/impl_alloc.rs:269:27
   9: bincode::features::impl_alloc::<impl bincode::de::Decode<Context> for alloc::string::String>::decode
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/impl_alloc.rs:341:21
  10: <bincode::features::serde::de_borrowed::SerdeDecoder<DE> as serde::de::Deserializer>::deserialize_string
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_borrowed.rs:220:30
  11: serde::de::impls::<impl serde::de::Deserialize for alloc::string::String>::deserialize
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.219/src/de/impls.rs:704:9
  12: <core::marker::PhantomData<T> as serde::de::DeserializeSeed>::deserialize
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.219/src/de/mod.rs:800:9
  13: <<bincode::features::serde::de_borrowed::SerdeDecoder<DE> as serde::de::Deserializer>::deserialize_tuple::Access<DE> as serde::de::SeqAccess>::next_element_seed
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_borrowed.rs:314:33
  14: serde::de::SeqAccess::next_element
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.219/src/de/mod.rs:1734:9
  15: <tests::models::allocated::_::<impl serde::de::Deserialize for tests::models::allocated::Allocated>::deserialize::__Visitor as serde::de::Visitor>::visit_seq
             at ./tests/src/models/allocated.rs:18:56
  16: <bincode::features::serde::de_borrowed::SerdeDecoder<DE> as serde::de::Deserializer>::deserialize_tuple
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_borrowed.rs:332:9
  17: <bincode::features::serde::de_borrowed::SerdeDecoder<DE> as serde::de::Deserializer>::deserialize_struct
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_borrowed.rs:417:9
  18: tests::models::allocated::_::<impl serde::de::Deserialize for tests::models::allocated::Allocated>::deserialize
             at ./tests/src/models/allocated.rs:18:56
  19: bincode::features::serde::de_borrowed::borrow_decode_from_slice
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_borrowed.rs:60:18
  20: bincode::features::serde::de_owned::decode_from_slice
             at /home/udoprog/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bincode-2.0.1/src/features/serde/de_owned.rs:66:5
  21: tests::utils::full::bincode_serde::decode::decode
             at ./tests/src/utils/full.rs:68:26
  22: tests::utils::full::bincode_serde::decode
             at ./tests/src/utils/full.rs:36:1
  23: fuzz::main
             at ./tests/src/bin/fuzz.rs:361:5
  24: core::ops::function::FnOnce::call_once
             at /home/udoprog/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The culprit is the decode implementation for containers like Vec<T>, where we decode the length that is expected to be read, and call Vec::with_capacity with the argument. This results in a vector of the provided length being allocated eagerly - even if there is no data to fill it. Meaning as little as an 8 byte payload or less of untrusted or corrupted payload can crash a server using bincode that is not configured with a limit*.

The broadly accepted solution is to limit the allocated capacity and decode the vector piecemeal, like here where I limit the allocated capacity in musli or the equivalent code in serde (which is more defensive taking the in-memory element size into account).

The thinking here is that legitimately large arrays will be filled with data from whatever buffer is being decoded. In order to allow the array to be filled, the buffer being decoded itself has to be large, which means that the source legitimately has to store or transmit that amount of data for it to be accepted which prevents at least small payloads from exhausting the resources of a server. Larger payloads would have to be explicitly accepted first before being decoded.

Note this only happened when I bumped to bincode 2.x. I did not see it in bincode 1.x. I think this was prevented by serde then.

[ZoeyR]

Generally we expected this to be prevented in bincode by configuring a byte limit with the Configuration struct. https://docs.rs/bincode/latest/bincode/config/struct.Configuration.html#method.with_limit. I can, however, see a good reason to also limit the eager allocation as well. Thanks for the report!