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 <statel...@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.
