[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110823185546.0TTF2.1506046.imail@eastrmwml48>
Date: Tue, 23 Aug 2011 18:55:46 -0400
From: <jfoug@....net>
To: john-dev@...ts.openwall.com
Cc: magnum <rawsmooth@...dband.net>
Subject: Re: valgrind vs rules
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.
