Keats
commented
1 week ago I'm aware there's some "precedent" with other implementations of bcrypt silently truncating the input to 72 bytes, but I would argue those implementations are similarly vulnerable to second preimage attacks. https://github.com/Keats/rust-bcrypt/issues/86 is a real-world example that this behavior is unexpected.
It's pretty much every implementation that truncates. Sometimes they have an argument to error on len > 72 but it's not the default. Making it an error would mean this crate would error on passwords that are valid in pretty much all other programming languages so that's not going to happen. We could add another function that errors when len > 72 if needed but that wouldn't be the default.
Per #86 and the following link, only the first 72 bytes of the input are considered when generating a hash, leading to hash collisions in the case that inputs are 73 bytes or longer:
https://github.com/Keats/rust-bcrypt/blob/9c9e138/src/lib.rs#L117-L119
(Sidebar: the comment seems to suggest a potential panic when computing a hash for an input longer than 72 bytes, which is a potential DoS vulnerability)
I would suggest returning a
Result-based error from both functions in the event the input is longer than 72 bytes.I'm aware there's some "precedent" with other implementations of bcrypt silently truncating the input to 72 bytes, but I would argue those implementations are similarly vulnerable to second preimage attacks. #86 is a real-world example that this behavior is unexpected.