Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5660A0DA.4070603@openwall.net>
Date: Thu, 3 Dec 2015 14:06:50 -0600
From: jfoug <jfoug@...nwall.net>
To: john-dev@...ts.openwall.com
Subject: Re: rules.c patch for ASan fault

How about this change:

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)((unsigned 
char)(rules_vars['m'] + 1)) - mpos;
                                 if (count > mleft)
                                         count = mleft;
                                 if (count <= 0)

On 12/3/2015 12:14 PM, magnum wrote:
> Solar,
>
> 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. This leads to an ASan fault (at least a 
> "read" fault) 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.
>
> For background, see 
> https://github.com/magnumripper/JohnTheRipper/issues/1744
>
> 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.