Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <554A0372.80303@mailbox.org>
Date: Wed, 06 May 2015 14:05:06 +0200
From: Frank Dittrich <frank.dittrich@...lbox.org>
To: john-dev@...ts.openwall.com
Subject: Re: Undefined behavior in bench.c (null pointer passed
 as argument 1 to memcpy)

On 05/06/2015 12:24 PM, Solar Designer wrote:
> On Wed, May 06, 2015 at 02:18:17AM +0200, Frank Dittrich wrote:
>> bench.c line 138 is:
>>
>> 		memcpy(two_salts[index], salt, format->params.salt_size);
>>
>> and, apparently, two_salts[index] is NULL here (and for other saltless
>> formats).
>>
>> Since this is core code, magnum would obviously prefer a core change
>> over a jumbo specific change.
> 
> Yes, this is a core bug.  The problem is that our mem_alloc() returns
> NULL when size is 0, but the code in bench.c proceeds to use memcpy(),
> which isn't guaranteed to accept NULL even when size is 0.  We need to
> either change our mem_alloc() to be similar to malloc() in this respect
> (which might increase John's memory usage), or change the code in
> bench.c (and I wonder if there are more instances of this issue).

I don't think there are more of these issues.
Recently I checked jumbo, and this was the only such problem.

Now I tested a core build with OpenMP enabled and
-fsanitize=nonnull-attribute and -fsanitize-undefined-trap-on-error
(added to CFLAGS and LDFLAGS).

While
$ ./john --test --format=LM
now results in "Illegal instruction (core dumped)" and exit code 132,
repeated runs of the test suite (with and without --fork) and with
various OMP_NUM_THREAD values didn't show any problems.

So it really is this one problem with --test for saltless formats.

I'll repeat the complete test with all the other options detecting
undefined behavior and report the results.

>> For -fsanitize=alignment, there are error messages for MD5_std.c, lines
>> 745-752: store to misaligned address ... for type "MD5_word', which
>> requires 4 byte alignment.
> 
> We deliberately use unaligned accesses in MD5_std.c when
> ARCH_ALLOWS_UNALIGNED.  You can try building without
> ARCH_ALLOWS_UNALIGNED and see if these messages are gone (I'd be very
> surprised if they are not).

I'll test that as well.

Frank

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.