Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160706163847.GA4860@openwall.com>
Date: Wed, 6 Jul 2016 19:38:47 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: rules.c bug/feature

On Wed, Jul 06, 2016 at 05:27:33PM +0200, magnum wrote:
>                 case 'd':
> -                       memcpy(in + length, in, length);
> +                       if (rules_max_length - length > 0)
> +                               strnzcpy(in + length, in,
> +                                        rules_max_length - length);

In addition to what I just wrote, the above patch looks wrong to me in
its use of rules_max_length.  That's hash type dependent maximum length,
whereas the word may reasonably be longer than that inbetween commands.

While looking into our uses of rules_max_length, I just noticed that we
might have a bug in the 'X' command, where it doesn't take into account
that 'M' only stores up to rules_max_length yet records the potentially
larger length into the 'm' variable.  Oops.  'M' was implemented that
way since it's all that 'Q' needed - although I'm afraid this aspect of
behavior of 'Q' is undocumented, and arguably is wrong, despite of being
intentional.  It looks like now we need to either revise 'M' to store
the entire word, or to revise 'X' (and update its documentation) so that
it doesn't try to restore beyond rules_max_length (but that would be
weird, right?)  We could also want to revise 'Q' to compare the entire
word, or we need to document its current behavior.

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.