Re: [dev] [sbase] [patch v2] Add md5sum

From: sin <sin_AT_2f30.org>
Date: Wed, 19 Jun 2013 15:00:43 +0300

On Wed, Jun 19, 2013 at 02:55:12PM +0300, sin wrote:
> On Wed, Jun 19, 2013 at 12:51:33PM +0200, Szabolcs Nagy wrote:
> > * stateless <stateless_AT_archlinux.us> [2013-06-19 11:38:00 +0100]:
> > > This is a version of md5sum(1) using the md5 routines from 9base - slightly
> > > adapted to compile.
> > >
> >
> > be careful with integer arithmetics in crypto code
> >
> > your code invokes undefined behaviour because of
> > signed int overflow:
> >
> > unsigned f(unsigned char c) { return c<<24; }
> >
> > c is promoted to int not unsigned in the left shift
> >
> > it will work in practice (usually) but that's only
> > by accident
> >
> > you should cast c to the right type
>
> You are right. I will send some incremental patches
> to fix this (and for 9base too).
> I presume this is in the utils/md5*.c code which I have not
> looked at in anger yet.
>
> Integer promotion rules are nasty! I think something like
> the following would still be ok?
>
> unsigned f(unsigned int c) { return c<<24U; }

Although in this case we still have undefined behaviour
because unsigned int can be 2 bytes by the standard.

Depending on the ABI this might or might not be an issue.
Received on Wed Jun 19 2013 - 14:00:43 CEST