BasicDecoder Panics when Decoding Specially Crafted Byte Stream

[onicslabs]

BasicDecoder panics when an UntrustedRlp struct encodes the following byte stream:
\xbf\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xe5

Here is the full backtrace and libFuzzer output for the crash:

[4] thread '<unnamed>' panicked at 'attempt to add with overflow', src/untrusted_rlp.rs:374:31
[4] stack backtrace:
[4]    0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
[4]              at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
[4]    1: std::sys_common::backtrace::_print
[4]              at /checkout/src/libstd/sys_common/backtrace.rs:71
[4]    2: std::panicking::default_hook::{{closure}}
[4]              at /checkout/src/libstd/sys_common/backtrace.rs:60
[4]              at /checkout/src/libstd/panicking.rs:380
[4]    3: std::panicking::default_hook
[4]              at /checkout/src/libstd/panicking.rs:396
[4]    4: std::panicking::rust_panic_with_hook
[4]              at /checkout/src/libstd/panicking.rs:611
[4]    5: std::panicking::begin_panic_new
[4]              at /checkout/src/libstd/panicking.rs:553
[4]    6: std::panicking::begin_panic_fmt
[4]              at /checkout/src/libstd/panicking.rs:521
[4]    7: rust_begin_unwind
[4]              at /checkout/src/libstd/panicking.rs:497
[4]    8: core::panicking::panic_fmt
[4]              at /checkout/src/libcore/panicking.rs:92
[4]    9: core::panicking::panic
[4]              at /checkout/src/libcore/panicking.rs:51
[4]   10: rlp::untrusted_rlp::BasicDecoder::decode_value
[4]              at src/untrusted_rlp.rs:374
[4]   11: rlp::impls::<impl rlp::traits::Decodable for u8>::decode
[4]              at src/impls.rs:112
[4]   12: rlp::untrusted_rlp::UntrustedRlp::as_val
[4]              at ./src/untrusted_rlp.rs:248
[4]   13: rust_fuzzer_test_input
[4]              at fuzz/fuzz_targets/untrusted_data.rs:15
[4]   14: libfuzzer_sys::test_input_wrap::{{closure}}
[4]              at .cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/67f7399/src/lib.rs:11
[4]   15: std::panicking::try::do_call
[4]              at /checkout/src/libstd/panicking.rs:479
[4]   16: <unknown>
[4]              at /checkout/src/libpanic_abort/lib.rs:40
[4] ==10120== ERROR: libFuzzer: deadly signal
[4]     #0 0x55eea7179ef3  (parity/util/rlp/fuzz/target/x86_64-unknown-linux-gnu/debug/untrusted_data+0x1f5ef3)
[4]     #1 0x55eea6fda0db  (parity/util/rlp/fuzz/target/x86_64-unknown-linux-gnu/debug/untrusted_data+0x560db)
[4]     #2 0x55eea6fda025  (parity/util/rlp/fuzz/target/x86_64-unknown-linux-gnu/debug/untrusted_data+0x56025)
[4]     #3 0x55eea6fe759a  (parity/util/rlp/fuzz/target/x86_64-unknown-linux-gnu/debug/untrusted_data+0x6359a)
[4]     #4 0x7fa449a5e7df  (/usr/lib/libpthread.so.0+0x117df)
[4]     #5 0x7fa4494c38bf  (/usr/lib/libc.so.6+0x338bf)
[4]     #6 0x7fa4494c4f71  (/usr/lib/libc.so.6+0x34f71)
[4]     #7 0x55eea70ab578  (parity/util/rlp/fuzz/target/x86_64-unknown-linux-gnu/debug/untrusted_data+0x127578)
[4]
[4] NOTE: libFuzzer has rudimentary signal handlers.
[4]       Combine libFuzzer with AddressSanitizer or similar for better crash reports.
[4] SUMMARY: libFuzzer: deadly signal
[4] MS: 5 ChangeByte-EraseBytes-CopyPart-InsertRepeatedBytes-ChangeBit-; base unit: 29a8847c038f447209d8b5921d767e8da5aa1221
[4] 0xbf,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xe5,
[4] \xbf\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xe5
[4] artifact_prefix='parity/util/rlp/fuzz/artifacts/untrusted_data/'; Test unit written to parity/util/rlp/fuzz/artifacts/untrusted_data/crash-66fa3c8afdf2f0feb5fbbbd82ba7fd23a672812a
[4] Base64: v///////////////5Q==
[4] ==10120==LeakSanitizer has encountered a fatal error.
[4] ==10120==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
[4] ==10120==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
[4] MS: 5 ChangeByte-EraseBytes-CopyPart-InsertRepeatedBytes-ChangeBit-; base unit: 29a8847c038f447209d8b5921d767e8da5aa1221
[4] 0xbf,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xe5,
[4] \xbf\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xe5
[4] Base64: v///////////////5Q==

The important bit called right before the panic appears to be a line in parity/util/rlp/src/untrusted_rlp.rs:374:

let last_index_of_value = begin_of_value + len;

begin_of_value and len are both usize variables, so the addition appears to be causing the panic. Going to try testing this with the standard compiler (cargo-fuzz requires nightly to run), and see if the bug still pops up.

[onicslabs]

Confirmed panic also occurs under a stable build. Will push latest code as a commit to the fuzz testing pull request.

Just a simple binary that will compile under standard, and confirms the panic. Also posting below for convenience:

extern crate rlp;

use rlp::{DecoderError, UntrustedRlp};

fn main() {
    let bs = [0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe5]; 
    let rlp = UntrustedRlp::new(&bs);
    let _: Result<u8, DecoderError> = rlp.as_val(); 
}