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.