[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120207144139.GA31177@openwall.com>
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[0] - and indeed
these are used as array indices by generate(). init() also does not
bother to initialize anything but id[0], 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] = 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.
