Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 3 Dec 2015 21:28:05 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: rules.c patch for ASan fault

magnum,

On Thu, Dec 03, 2015 at 07:14:46PM +0100, magnum wrote:
> Here's a (maybe) proposed patch against john proper:
> 
> diff --git a/src/rules.c b/src/rules.c
> index 35cfe15..7eae64e 100644
> --- a/src/rules.c
> +++ b/src/rules.c
> @@ -825,7 +825,7 @@ char *rules_apply(char *word, char *rule, int split, 
> char *last)
>                                 POSITION(mpos)
>                                 POSITION(count)
>                                 POSITION(ipos)
> -                               mleft = (int)(rules_vars['m'] + 1) - mpos;
> +                               mleft = (int)(rules_vars['l']) - mpos;
>                                 if (count > mleft)
>                                         count = mleft;
>                                 if (count <= 0)
> 
> 
> This is within the 'X' command. The rationale is that rules_vars['m'] is 
> an unsigned char, initially set to (length - 1). When length is 0, 
> rules_vars['m'] is thus 255.

... but (rules_vars['m'] + 1) is then 0, isn't it?

> This leads to an ASan fault (at least a "read" fault)

I'll need to figure out why this is the case and how to fix that.

> unless this patch is applied. There doesn't seem to be any 
> more instance of similar problem.
> 
> Is there some intended behavior that this patch would break? I can't 
> imagine any.

Indeed, the patch looks wrong: the 'X' command is defined to extract
from memory, and it's the 'm' variable that holds the memorized word's
length.  Your patch might be correct for the case when the 'M' command
hasn't been used, but it probably is incorrect when 'M' has been used.

> For background, see 
> https://github.com/magnumripper/JohnTheRipper/issues/1744

Thank you for bringing this to my attention.

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.