Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 15 Sep 2015 01:22:28 +0300
From: Solar Designer <>
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

> 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.


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.