Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 6 May 2015 13:24:01 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Undefined behavior in bench.c (null pointer passed as argument 1 to memcpy)

Frank,

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).

http://www.tedunangst.com/flak/post/zero-size-objects

> I tested jumbo with all -fsanitize=undefined options except
> -fsanitize=alignment, which resulted in a huge number of false positives
> for x86, and this was the only error I found for all the other
> -fsanitize options related to undefined behavior.
> 
> 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).

Thanks,

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.