From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/1111 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: FreeSec crypt() Date: Wed, 13 Jun 2012 08:58:39 -0400 Message-ID: <20120613125839.GB163@brightrain.aerifal.cx> References: <20120612235113.GA21296@openwall.com> <20120613011842.GA163@brightrain.aerifal.cx> <20120613061032.GH17860@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1339592596 5168 80.91.229.3 (13 Jun 2012 13:03:16 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 13 Jun 2012 13:03:16 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-1112-gllmg-musl=m.gmane.org@lists.openwall.com Wed Jun 13 15:03:15 2012 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1SenE1-00051P-3P for gllmg-musl@plane.gmane.org; Wed, 13 Jun 2012 15:03:05 +0200 Original-Received: (qmail 24307 invoked by uid 550); 13 Jun 2012 13:03:04 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 24290 invoked from network); 13 Jun 2012 13:03:04 -0000 Content-Disposition: inline In-Reply-To: <20120613061032.GH17860@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:1111 Archived-At: 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