Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150712141803.GA2550@openwall.com>
Date: Sun, 12 Jul 2015 17:18:03 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: more robustness

Hi Kai,

On Sun, Jul 12, 2015 at 07:46:48PM +0800, Kai Zhao wrote:
> To mimic the real cracking process, I tried to change the loader.c to reuse
> for fuzzing. The last three commits reuse loader.c for fuzzing.
> 
> https://github.com/loverszhaokai/JohnTheRipper/commit/c4a3fc5c177d5a4181ca5cb3d2b72de95ab8818e
> https://github.com/loverszhaokai/JohnTheRipper/commit/6300f5fae0713e740169250877a67ab9380ce71a
> https://github.com/loverszhaokai/JohnTheRipper/commit/f8a6f01a12e47cb9d951a7733fa0a69af1bd6204

I'm not happy that you're making any changes to loader.c at all, but the
changes are relatively small, so this may be acceptable.

magnum - please take a look, I think these changes are more in your area.

[solar@...er fuzz_option]$ git diff cc6012eaf61c67800da7ef33b44fa7f19fa5d631 | diffstat 
 run/fuzz.dic       |   76 ++++++
 run/fuzz_option.pl |  155 +++++++++++++
 src/Makefile.in    |    4 
 src/fuzz.c         |  594 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/fuzz.h         |   11 
 src/john.c         |    6 
 src/loader.c       |   52 ++++
 src/loader.h       |    5 
 src/options.c      |    8 
 src/options.h      |   15 +
 10 files changed, 920 insertions(+), 6 deletions(-)

> After reuse loader.c::ldr_load_pw_line(), --fuzz now fuzz those functions:
> prepare(), valid(), init(), split(), binary(), salt(), salt_hash().
> However, there
> are some functions in crack.c before crypt_all(), such as set_salt(),
> clear_keys(), set_key(). **Should I fuzz these functions ? **

Since in some formats we're effectively starting the hashing in
set_salt() or/and set_key(), I think it'd only be consistent to fuzz
those if we also proceeded with crypt_all().

So for now the answer is no.

> There are 4 bugs found by the latest --fuzz.
> 
> https://github.com/loverszhaokai/JohnTheRipper/tree/fuzz_option
> 
> Bugs are below:
> 
> https://github.com/magnumripper/JohnTheRipper/issues/1548
> https://github.com/magnumripper/JohnTheRipper/issues/1547
> https://github.com/magnumripper/JohnTheRipper/issues/1546
> https://github.com/magnumripper/JohnTheRipper/issues/1545

OK, although this brings up the question: why were not these found by
fuzzing earlier, prior to --fuzz option?

Unrelated, here's a task for you for next week: identify improperly set
or missing FMT_* flags.  For example, a format supporting 8-bit chars in
passwords (unlike descrypt, which drops the 8th bit, by its definition),
but forgetting to set FMT_8_BIT.  Or vice versa.  Ditto about FMT_CASE,
FMT_OMP, etc.  One of the trickier flags is FMT_SPLIT_UNIFIES_CASE, and
even trickier is split() actually needing to do this in some cases.
Maybe magnum will help you figure these out.  (My availability will
likely be too limited, unfortunately.)

Maybe you can even write a script that would spot some of the likely
improper flag (non-)uses.  e.g. a _fmt*.c file mentions OpenMP stuff,
but never mentions FMT_OMP, or vice versa.  Some of this could be easier
detected at runtime - e.g., "\x20" and "\xa0" hashing differently, but a
format lacks FMT_8_BIT, or vice versa.  Your builtin fuzzer or extended
self-test could detect that.

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.