libiberty sha1.c: bug in adding 64-bit number to 64-bit number (binutils-2.22.90)

libiberty sha1.c: bug in adding 64-bit number to 64-bit number (binutils-2.22.90)

[Geoff Pike]

Hello libiberty experts,

I don't see anything saying that sha1_process_block() has a size limit
on its input buffer, and if the length of the buffer is big (e.g.,
2^32 on a 64-bit machine) then this code won't correctly add a 64-bit
number to 64-bit number:

  /* First increment the byte count.  RFC 1321 specifies the possible
     length of the file up to 2^64 bits.  Here we only compute the
     number of bytes.  Do a double word increment.  */
  ctx->total[0] += len;
  if (ctx->total[0] < len)
    ++ctx->total[1];

The above is at sha1.c around line 302.

regards,

Geoff Pike

Re: libiberty sha1.c: bug in adding 64-bit number to 64-bit number (binutils-2.22.90)

[Florian Weimer]

On 09/18/2012 02:13 AM, Geoff Pike wrote:
> Hello libiberty experts,
>
> I don't see anything saying that sha1_process_block() has a size limit
> on its input buffer, and if the length of the buffer is big (e.g.,
> 2^32 on a 64-bit machine) then this code won't correctly add a 64-bit
> number to 64-bit number:
>
>    /* First increment the byte count.  RFC 1321 specifies the possible
>       length of the file up to 2^64 bits.  Here we only compute the
>       number of bytes.  Do a double word increment.  */
>    ctx->total[0] += len;
>    if (ctx->total[0] < len)
>      ++ctx->total[1];
>
> The above is at sha1.c around line 302.

I'm not sure if many people read the gcc-bugs list, it's mainly fed from 
Bugzilla.  Better file a bug.

After a quick look at the code, I'm wondering if the ~63 is properly 
sign-extended and then converted to size_t:

	  sha1_process_block (buffer, len & ~63, ctx);

This should work, but it looks a bit fishy.

-- 
Florian Weimer / Red Hat Product Security Team