Possible regression of "BSD wraparound" bug?

[ivan-c]

passlib implements a specific check against the "BSD wraparound" bug that now appears to be failing with bcrypt 3.0

Although the included source from libcrypt appears to have been updated since the bug was fixed (OpenBSD 5.5, according to passlib) I'm not familiar enough with the code to be able to tell if this is a genuine regression or not

[charmander]

The check uses a $2a$ hash where the behaviour is maintained for compatibility:

switch ((minor = salt[1])) {
case 'a':
    key_len = (u_int8_t)(strlen(key) + 1);
    break;
case 'b':
    /* strlen() returns a size_t, but the function calls
     * below result in implicit casts to a narrower integer
     * type, so cap key_len at the actual maximum supported
     * length here to avoid integer wraparound */
    key_len = strlen(key);
    if (key_len > 72)
        key_len = 72;
    key_len++; /* include the NUL */
    break;
⋮

[dstufft]

I think maintaining compatibility for this is kind of bonkers, but in any case, prior to 3.0 we had the fix even for $2a in this library, so we should restore said fix.

[reaperhulk]

@dstufft do you have an opinion on how we should fix this to have it behave the same as bcrypt 2.0.0? Could we take all input passwords and do password = password[:72]?

[dstufft]

Truncating to 72 seems reasonable, since that's all the C code is going to do anyways in the correct case.