password_compat
password_compat copied to clipboard
Several issues with custom salt
If the custom salt contains a character which isn't a valid bcrypt Base64 digit, the library will silently encode the salt. This is a huge problem. If the salt is just a faulty Base64 string (e. g., it uses +
instead of .
), it will be double-encoded, causing it to effectively lose 33 bits.
The library also requires the custom salt to always contain 22 bytes, even if a raw salt would only have to be 16 bytes long. This forces us to either generate 6 useless extra bytes or pad the string with garbage data.
Unfortunately, there's no simple fix, because it's not always possible to distinguish between a raw salt and a pre-encoded one. If the string happens to contain only Base64 digits, it could be both.
I suggest to either ask for raw salts or for pre-encoded salts, not both. Using raw salts is probably the better solution:
- Why would anybody want to do the weird Base64 encoding herself?
- This is also very error-prone. Many people aren't aware that bcrypt doesn't use standard Base64.
- Base64 encoded salts require complicated validation (the length, the digits and a special check for the last digit). With raw salts, we only have to check the length.
Either way, it should be only one possible format, and the manual should clearly state what the function expects.
The main bug as I see it, is that is not possible to pass a custom salt that is 16 bytes long.
If you want to use your own salt, in the current implementation you needlessly have to pad it to 22 bytes.