[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130508034135.GA23322@openwall.com>
Date: Wed, 8 May 2013 07:41:35 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Core: undefined behavior in DES_std.c and MD5_std.c
On Tue, May 07, 2013 at 11:26:50PM +0400, Alexander Cherepanov wrote:
> I've tried to compile core john with clang -fsanitize=undefined and have
> got several errors (on x86_64).
>
> in bsdicrypt:
> DES_std.c:407:18: runtime error: left shift of 1 by 63 places cannot be
> represented in type 'long'
[...]
> The problem is that trying to compute 1<<31 or (long)1<<63 leads to an
> undefined behavior (per C11, 6.5.7p4). A tentative patch (for these and
> some more) is attached
I've just reviewed and applied this patch. Thanks!
> but it seems a thorough check of all the file is
> due because it freely mixes signed and unsigned types.
It is one of the oldest files in JtR, and I am fully aware of this
shortcoming. Other files are also affected - even some new code in
them, where I don't yet deviate from the approach that had been taken in
those same files.
> Is it really necessary to have ARCH_WORD signed?
I think there are a few places that require a signed type like this,
e.g. DES_bs_cmp_all(). BTW, its "~(ARCH_WORD)0" should probably be
"~(unsigned ARCH_WORD)0", its right shifts could be turned from signed
(usually arithmetic) to unsigned (logical) ones and the value only then
cast to a signed type for the negation.
All of this is just asking for a rewrite. Some years ago I actually
thought of rewriting JtR from scratch, but I never found time.
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.
