Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 8 Jun 2015 14:27:19 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Fuzzing Report on hashes

Kai,

On Sun, Jun 07, 2015 at 11:01:49PM +0800, Kai Zhao wrote:
> I do not understand the false positives.
[...]
> So the problem is that john reports "1234567890" is the password ?

In your example, yes.  It shouldn't have cracked any password, because
the hash was modified such that it is likely infeasible to find a
password that would match it.  And if the hash were not modified, then
the correct password would have been "realmenuseJtR" assuming that the
test vectors in django_scrypt_fmt_plug.c are correct.

Looking at the code, I think the problem is that crypt_all() ignores the
return value from crypto_scrypt().  This may result in the test vectors'
hashes being left in crypt_out[] from the self-tests.  When the hash
we're cracking is also from among those same test vectors, it is not
surprising that it matches one of those.  This also explains why I
stopped detecting this problem when I added --skip-self-tests.  And this
suggests that it makes some to do some fuzzing without --skip-self-tests
as well.

The quickest fix should be to add a crypto_scrypt() return value check
to django_scrypt_fmt_plug.c: crypt_all() and have it zeroize the
corresponding crypt_out[] element (the full BINARY_SIZE of it) on
failure.  Maybe a warning should be printed as well, or maybe this
should be treated as a fatal error and the program should exit.

As I mentioned before, a better fix is to drop django_scrypt_fmt_plug.c
and merge its functionality into scrypt_fmt.c if it's not already in
there.  django_scrypt_fmt_plug.c is also unnecessarily slower, using the
interface that keeps the memory (de)allocation overhead in the loop.

BTW, I've just checked: scrypt_fmt.c: crypt_all() does handle error
returns from escrypt_r(), treating them as fatal errors.  Because of use
of OpenMP, the error exit is postponed until the parallel for loop
terminates, though.  You could want to add a similar fix for
django_scrypt_fmt_plug.c for now.

Please enter this into a GitHub issue.  Also, please create another
GitHub issue for the openssl_enc_fmt_plug.c false positive.  From a
quick look at openssl_enc_fmt_plug.c, I don't immediately see what the
bug is - you might want to look into it on your own.

Thanks,

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.