|| ||Solar Designer <solar-cxoSlKxDwOJWk0Htik3J/w-AT-public.gmane.org> |
|| ||oss-security-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8-AT-public.gmane.org |
|| ||CVE request: crypt_blowfish 8-bit character mishandling |
|| ||Mon, 20 Jun 2011 09:01:11 +0400|
|| ||magnum <rawsmooth-cgr7CL/LOSDk1uMJSBkQmQ-AT-public.gmane.org>, Pierre Joye <pierre.php-Re5JQEeQqe8AvxtiuMwx3w-AT-public.gmane.org>|
|| ||Article, Thread
Earlier today, while working on a test suite for John the Ripper, magnum
discovered and reported what turned out to be a bug in John the Ripper
The bug is inadvertent sign extension, and the fix is trivial:
This bug dates back to 1998 (or maybe even 1997).
Unfortunately, the bug is not only in JtR, but also in crypt_blowfish,
and thus in plenty of other systems and programs that have integrated
crypt_blowfish. Obviously, I am quite embarrassed; I should have
included 8-bit test vectors or subjected crypt_blowfish to a fuzzer (vs.
OpenBSD's implementation), or/and used different coding conventions (use
"unsigned char" almost everywhere, although this has its problems too -
such as compiler warnings on library calls that expect simple "char *").
Since the code successfully worked in JtR, I thought that it was
essentially already fuzz-tested. But apparently passwords with 8-bit
characters were uncommon enough that no one noticed the bug for years.
I am going to provide an official fix for crypt_blowfish (likely the
one-liner plus added tests). I thought I'd bring the issue up on
oss-security sooner rather than later.
Here's my preliminary analysis of the impact:
The majority of hashes (but not all of them) for passwords containing
characters with the 8th bit set are incompatible with OpenBSD's (really
nasty, but no security impact here).
What's worse, approximately 3 in 16 passwords containing a single
character with the 8th bit set have 1 to 3 characters immediately
preceding the 8-bit character ignored. With more than one character
with the 8th bit set, things may be even worse.
Thus, those passwords may be much easier to crack than expected.
As to what's affected besides crypt_blowfish itself, I expect it to be
PHP (the code in php-5.3.7RC1 looks affected), Linux distros that use
crypt_blowfish (Owl, ALT Linux, SUSE), and some others (I'll try to
identify them and notify the maintainers).
Sorry about that!
Since this is the second bug with char signedness in crypt_blowfish, it
looks like I have a lesson to learn here. The last time, the bug was
with salt generation for hash types other than bcrypt (that code was
little used and little tested). Besides fixing the bug, I responded by
running extensive tests and making sure the distribution of salts was
uniform. Of course, it was better to run those tests before releasing
the code to the public. Now we have an issue with the passwords
themselves. Obviously, I will be adding more tests, even though it
would be better done before releasing the code.
No, I don't expect even more sign extension bugs in crypt_blowfish.
There's not that much code, and we've pretty much tested it by now.
However, I might reconsider my C programming conventions for new code as
it relates to use of integer types. I think I'd rather workaround
meaningless compiler warnings on strlen() and the like (even though
those extra casts look dirty) than miss real bugs elsewhere.
Perl's Crypt::Eksblowfish turns out to have sufficiently reworked code
that it's unaffected:
Oh, also some builds of crypt_blowfish (and of affected systems/apps)
for PowerPC are probably unaffected, because char is typically unsigned
there (unless overridden in compiler flags for compatibility with more
Once again, my apologies for the mess.
to post comments)