[Pkg-shadow-devel] SHA_salt_size() problem

[Nicolas François]

Hello Peter,

On Tue, May 20, 2008 at 01:33:21PM +0200, pvrabec at redhat.com wrote:
> hi folks,
> 
> see https://bugzilla.redhat.com/show_bug.cgi?id=447136
> 
> There are two problems in libmisc/salt.c with:
> static unsigned int SHA_salt_size (void)
> {
>         double rand_rounds = 9 * random ();
>         rand_rounds /= RAND_MAX;
>         return 8 + rand_rounds;
> }
> 
> 1. random() is not init by srandom()
> 2. rand_rounds overflow
> 
> suggested fix:
> static unsigned int SHA_salt_size (void)
> {
> 	seedRNG ();
> 	unsigned int rand_rounds = random () % 9;
> 	return 8 + rand_rounds;
> }

Thanks a lot for this patch.

SHA_salt_rounds also suffer from the same overflow.

The overflow is caused by the computation which is done with the type of "9".

Forcing the computation to be done with double is sufficient:
         double rand_rounds = (double) 9.0 * random ();
         rand_rounds /= RAND_MAX;

It is recommended not to use a modulo for randomness (although it would
have been better than my buggy version). I don't really remember why (it
is mentioned in the srand manpage, but this may not apply for non power
of 2, and for small numbers), but I would prefer to keep this template.

The patch I applied is attached.

I consider the security implications quite low (the number of salt chars
will always be the same as what the MD5 algorithm provides, and the lowest
number of rounds should be acceptable).
Thus I will not issue a security warning.

I'm considering releasing 4.1.2, which would include this fix, a few other
fixes backported from Debian patches, and some documentation fixes.

Debian is currently in freeze for the next release, so I will probably not
package this 4.1.2 for Debian, but I will ask the release managers to
accept this patch and maybe the documentation fixes.

Best Regards,
-- 
Nekral
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Fedora_447136.diff
Type: text/x-diff
Size: 2494 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-shadow-devel/attachments/20080520/296db255/attachment.diff