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 = key & 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 > 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.