[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 15 Sep 2015 01:22:28 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Invalid memory our of bounds read in DES_std_set_key
Hanno, magnum -
On Sun, Sep 13, 2015 at 05:12:51PM +0200, Hanno B??ck wrote:
> When compiling john-1.8.0 with address sanitizer it will expose an out
> of bounds read in the function DES_std_set_key.
>
> The error happens in line 664:
> DES_key[1] = key[1] & 0x7F;
>
> The variable "key" is a parameter to the function and the value comes
> indirectly from a function call in formats.c, line 168:
> format->methods.set_key("", index);
>
> As you can see an empty string is passed. Therefore accessing key[1]
> won't work.
Thank you!
I've just fixed this in the core tree by documenting set_key() as
potentially over-reading for up to PLAINTEXT_BUFFER_SIZE total, and by
providing fmt_null_key for such use (similar to how jumbo already has
safe_null_key in inc.c, which should now be replaced with use of
fmt_null_key).
> I'm not entirely sure what was the intention of that code, so I'm not
> sure how to fix it.
The intention was to quickly process a descrypt password of up to 8
characters long, and most likely closer to that maximum length.
Checking each char for NUL is slower.
While DES_std.c is almost obsolete now, I think we have other similar
occurrences in some formats in jumbo, which is why I chose to fix the
issue the way I did (as described above).
Ideally, we should declare fmt_null_key with the const qualifier, but
this requires that we make set_key()'s argument const as well, and
that's a change that should propagate to all jumbo formats (a few
hundred by now), so we better make it separately, as part of an even
bigger const'ification effort.
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.
