Panic when verifying with malformed hash

5225225 commented 3 years ago

fn main() {
    bcrypt::verify(&[], "2a$$$0$OOOOOOOOOOOOOOOOOOOOO£OOOOOOOOOOOOOOOOOOOOOOOOOOOOOO");
}

gives a stack trace of

thread 'main' panicked at 'byte index 22 is not a char boundary; it is inside '£' (bytes 21..23) of `OOOOOOOOOOOOOOOOOOOOO£OOOOOOOOOOOOOOOOOOOOOOOOOOOOOO`', /home/jess/.cargo/registry/src/github.com-1ecc6299db9ec823/bcrypt-0.10.0/src/lib.rs:150:22
stack backtrace:
   0: rust_begin_unwind
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/core/src/panicking.rs:92:14
   2: core::str::slice_error_fail
   3: core::str::traits::<impl core::slice::index::SliceIndex<str> for core::ops::range::RangeTo<usize>>::index
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/core/src/str/traits.rs:289:21
   4: core::str::traits::<impl core::ops::index::Index<I> for str>::index
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/core/src/str/traits.rs:64:9
   5: bcrypt::split_hash
             at /home/jess/.cargo/registry/src/github.com-1ecc6299db9ec823/bcrypt-0.10.0/src/lib.rs:150:22
   6: bcrypt::verify
             at /home/jess/.cargo/registry/src/github.com-1ecc6299db9ec823/bcrypt-0.10.0/src/lib.rs:193:17
   7: scratchhTyoWr8OM::main
             at ./main.rs:2:5
   8: core::ops::function::FnOnce::call_once
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/core/src/ops/function.rs:227:5

A malformed hash seems unlikely as an DoS vector, but the code seems to be checking for other forms of errors in the hash, so it shouldn't panic here.

Keats commented 3 years ago

Thanks for reporting it!

5225225 commented 3 years ago

by the way, you don't need the UTF-8 check in the fuzzer, you can

fuzz_target!(|data: &str| {
    let _ = bcrypt::hash(data, 4);
});

the type just needs to implement arbitrary::Arbitrary which &str does. (See: https://rust-fuzz.github.io/book/cargo-fuzz/structure-aware-fuzzing.html)

This also speeds up the fuzzing since on UTF-8 error, it can still return some amount of the &str, it doesn't completely waste that input.

Keats commented 3 years ago

Oh I didn't know that, thanks!

Keats / rust-bcrypt

Panic when verifying with malformed hash #62