[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 13 Jun 2012 08:58:39 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: FreeSec crypt()
On Wed, Jun 13, 2012 at 08:10:32AM +0200, Szabolcs Nagy wrote:
> > char *
> > _crypt_extended_r(const char *key, const char *setting, char *output)
> > {
> ....
> > while (q - (u_char *) keybuf < sizeof(keybuf)) {
> > *q++ = *key << 1;
>
> implementation-defined signed shift
Undefined. Right shifts are implementation-defined. Left-shifts are
defined only for positive operands that don't overflow.
Note that even if the behavior were defined, this code seems to have
different behavior for high bytes depending on the signedness of char.
That looks like a major bug; I thought that bug in crypt
implementations was fixed years ago.
> > for (i = 1, count = 0; i < 5; i++) {
> > int value = ascii_to_bin(setting[i]);
> > if (ascii64[value] != setting[i])
> > return NULL;
> > count |= value << (i - 1) * 6;
> > }
>
> signed shift (harmless)
Yes this one seems to be harmless unless it can overflow, and from the
loop bounds it seems impossible to overflow.
> > while (q - (u_char *) keybuf < sizeof(keybuf) && *key)
> > *q++ ^= *key++ << 1;
>
> signed shift
This one looks like it has the same bug as the first.
Rich
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.
