Follow @Openwall on Twitter for new release announcements and other news
[<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.