[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4E54313D.9090700@bredband.net>
Date: Wed, 24 Aug 2011 01:01:17 +0200
From: magnum <rawsmooth@...dband.net>
To: john-dev@...ts.openwall.com
Subject: Re: valgrind vs rules
I tried the same thing. But I suspect we should actually memset it to
non-zero. I think that test in rules.c is a shortcut for sometimes
skipping a memchk().
So I just dropped my fix and posted to john-dev a couple of days ago and
will let Solar decide.
magnum
On 2011-08-24 00:55, jfoug@....net wrote:
> This 'fixes' the warning (it is just a warning). It stems all the way back to wordlist.c This is the 'jumbo' code, not the core code, but I think the same issue is in core john. This does appear to be an issue. I wonder if 'last' needs to be memset each time, of if the single \0 is enough? It it is enough, then likely this bug would have likely only affected a single candidate word.
>
> Jim.
>
> void do_wordlist_crack(struct db_main *db, char *name, int rules)
> {
> union {
> char buffer[2][LINE_BUFFER_SIZE + CACHE_BANK_SHIFT];
> ARCH_WORD dummy;
> } aligned;
> const char *line = aligned.buffer[0];
> char *last = aligned.buffer[1];
> struct rpp_context ctx;
> char *prerule, *rule, *word;
> char *(*apply)(const char *word, char *rule, int split, char *last);
> long file_len;
> int i, pipe_input=0, max_pipe_words=0, rules_keep=0, init_this_time=1, really_done=0;
> char msg_buf[128];
>
> #ifdef HAVE_MPI
> char file_line[LINE_BUFFER_SIZE];
> long my_size = 0;
> unsigned int myWordFileLines = 0;
> #endif
>
> log_event("Proceeding with wordlist mode");
>
> _db = db;
>
> length = db->format->params.plaintext_length;
>
> + memset(&aligned, 0, sizeof(aligned));
>
> if (name) {
>
>
> ---- magnum<rawsmooth@...dband.net> wrote:
>> I get this from valgrind when running wordlist + rules (even in plain
>> 1.7.8, no jumbo):
>>
>> ==10714== Conditional jump or move depends on uninitialised value(s)
>> ==10714== at 0x426690: rules_apply (rules.c:917)
>> ==10714== by 0x42AC35: do_wordlist_crack (wordlist.c:218)
>> ==10714== by 0x420170: main (john.c:306)
>> ==10714== Uninitialised value was created by a stack allocation
>> ==10714== at 0x42AA6D: do_wordlist_crack (wordlist.c:133)
>>
>>
>> relevant part of rules.c:
>>
>> 905 out_OK:
>> 906 in[rules_max_length] = 0;
>> 907 if (last) {
>> 908 if (length> rules_max_length)
>> 909 length = rules_max_length;
>> 910 if (length>= ARCH_SIZE - 1) {
>> 911 if (*(ARCH_WORD *)in != *(ARCH_WORD *)last)
>> 912 return in;
>> 913 if (strcmp(&in[ARCH_SIZE - 1],&last[ARCH_SIZE - 1]))
>> 914 return in;
>> 915 return NULL;
>> 916 }
>> 917 if (last[length])
>> 918 return in;
>> 919 if (memcmp(in, last, length))
>> 920 return in;
>> 921 return NULL;
>> 922 }
>> 923 return in;
>>
>> length here is the length of the current word. As I understand it, if
>> the current word is longer than last has ever been, last[length] is
>> uninitialized - and this is what valgrind complains about. I'm not sure
>> I understand the purpose of line 917 at all so I'm not sure this is a
>> problem at all?
>>
>> magnum
>
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.
