Date: Tue, 7 Feb 2012 18:41:39 +0400 From: Solar Designer <solar@...nwall.com> To: john-dev@...ts.openwall.com Subject: Re: Was: RE: [john-users] Segmentation fault in john-1.7.9-jumbo-5 under some conditions On Mon, Feb 06, 2012 at 09:45:06AM -0600, jfoug wrote: > I am able to reproduce this under cygwin (which I have had very little > success debugging), but I can also have it happen under a VC build, which I > do have ability to debug. However, I am not comfortable in any knowledge > of your compiler code. I have written some myself, for gmp-ecm, OpenPGW, > and even a simple parser/compiler in pass_gen.pl. However, I have not dug > in, or figured out the compiler code in john. So I will list some > 'findings', and possibly we can track down just what is going on. I did not expect this would have anything to do with the compiler - I thought it'd be some jumbo-specific change. > op: [ This led me to suspect that we have an out of bounds access in the Keyboard external mode. Looking at it with this suspicion, I found that restore() does not bother to initialize id beyond id - and indeed these are used as array indices by generate(). init() also does not bother to initialize anything but id, but with the default minlength of 1 this did not matter. To reproduce the problem, I did: --- compiler.c.orig 2011-10-29 17:41:39 +0400 +++ compiler.c 2012-02-07 18:03:45 +0400 @@ -840,6 +840,7 @@ c_code_start = mem_alloc((size_t)c_code_ptr); c_data_start = mem_alloc((size_t)c_data_ptr); + memset(c_data_start, 'x', (size_t)c_data_ptr); } return c_errno; This worked - got a segfault when restoring a Keyboard mode session (using otherwise clean 1.7.9 on Linux). Then I patched the issue: --- john.conf.orig 2011-09-18 11:35:50 +0400 +++ john.conf 2012-02-07 18:14:52 +0400 @@ -604,8 +604,9 @@ } } - id = 0; - length = minlength; + length = 0; + while (length < minlength) + id[length++] = 0; } void generate() @@ -637,7 +638,8 @@ /* Calculate the length */ length = 0; - while (word[length]) length++; + while (word[length]) + id[length++] = 0; /* Infer the first character index */ i = -1; We need to get this fix into both main and jumbo trees. Also, we may consider making external mode behavior more deterministic when there are uninitialized variables - either initialize everything to zero (then the above two problems in Keyboard would not appear - or they could be considered valid uses of implied initialization to zero) or deliberately trigger problems like I did for testing. Jim - thank you, and sorry for spending your time on this whereas the bug turned out to be mine and something I could debug myself with just a little bit more 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.