Re: expat hash collision fix too predictable?

[Andreas Ericsson <ae () op5 se>]

On 04/05/2012 11:30 AM, Marcus Meissner wrote:
> Hi,
>
> while reviewing a expat regression (likely caused by the hash collision denial of service fix, but unclear)
> i stumbled about the randomness it uses.
>
>       static unsigned long
>       generate_hash_secret_salt(void)
>       {
>         unsigned int seed = time(NULL) % UINT_MAX;
>         srand(seed);
>         return rand();
>       }
>
> and it is seeded once at parser object creation.
>
> This is better than not seeding, but I am not sure if it is sufficient.

A pretty simple fix that makes it far better is to do

        struct timeval tv;
        unsigned int seed;

        gettimeofday(&tv, NULL);
        seed = (tv.tv_usec * 65531) % UINT_MAX;
        srand(seed);
        return rand();

The other option is ofcourse to not involve timestamps at all and
instead rely on a source with higher entropy, but this is usually
sufficient to make attacking it very unappealing. Especially when
considering that many xml docs contain a timestamp of when they were
generated, making the issue that much worse.

-- 
Andreas Ericsson                   andreas.ericsson () op5 se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.