Bug 14090 - md5_process_block() produces incorrect result with large block sizes
Summary: md5_process_block() produces incorrect result with large block sizes
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Andreas Jaeger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-10 13:41 UTC by Serge Belyshev
Modified: 2014-06-25 11:04 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
a testcase (1.33 KB, text/plain)
2012-05-18 15:28 UTC, Serge Belyshev
Details
fix length bug in md5_process_block (388 bytes, patch)
2012-05-18 20:22 UTC, Paul Eggert
Details | Diff
a similar patch to sha512.c (316 bytes, patch)
2012-05-24 16:12 UTC, Paul Eggert
Details | Diff
a slightly-improved patch to sha512.c (323 bytes, patch)
2012-05-24 16:20 UTC, Paul Eggert
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Serge Belyshev 2012-05-10 13:41:29 UTC
crypt/md5.c: md5_process_block() (lines 319..321) contains this code for total length handling. With large block sizes (ctx->total[0] + len) can exceed 8 GB, thus the total[1] needs to be incremented more than by one:

------------------------------------------------------------------------------
  /* 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];

------------------------------------------------------------------------------

This bug affects various copies of the same code located in gcc, sources and gnulib repositories, and probably others, though it is rather hard to trigger it: md5_process_block() is usually never fed with many gigabytes at once.
Comment 1 Serge Belyshev 2012-05-18 15:28:45 UTC
Created attachment 6411 [details]
a testcase

Testcase for the bug is attached.  It computes md5 sums for zero-filled blocks of various sizes from 0 to 16 GB and prints the result.
Comment 2 Paul Eggert 2012-05-18 20:22:43 UTC
Created attachment 6412 [details]
fix length bug in md5_process_block

Thanks for reporting this.  I fixed the similar bug in gnulib here:

http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=0403c76938c7f487d303818cd19a72a1b63eb94f

and I propose the attached (untested) patch, which is based
on the gnulib patch.
Comment 3 Andreas Jaeger 2012-05-19 12:11:02 UTC
In glibc I see only md5_process_block() hit by this bug. Serge, are there an other files in glibc?

Serge, thanks for the bugreport and Paul, thanks for the patch. I'm currently testing it and will try getting this into glibc.
Comment 4 Serge Belyshev 2012-05-19 13:11:20 UTC
(In reply to comment #3)
> In glibc I see only md5_process_block() hit by this bug. Serge, are there an
> other files in glibc?

In crypt/sha512.c:123:

#ifdef USE_TOTAL128
  ctx->total128 += len;
#else
  ctx->total[TOTAL128_low] += len;
  if (ctx->total[TOTAL128_low] < len)
    ++ctx->total[TOTAL128_high];
#endif

But only in remotely hypothetical case of !USE_TOTAL128 and 128-bit size_t and then only if someone actually calls sha512_process_block() with this huge buffer.

(and then again, surely there is some other code needs to be fixed before 128 word size will work)
Comment 5 Paul Eggert 2012-05-24 16:12:36 UTC
Created attachment 6419 [details]
a similar patch to sha512.c

OK, thanks, here's a similar (and similarly untested) patch to sha512.c.
Comment 6 Paul Eggert 2012-05-24 16:20:26 UTC
Created attachment 6420 [details]
a slightly-improved patch to sha512.c

This patch is a slightly-improved version of the earlier sha512.c patch.
Both patches are correct, but this one makes the code more parallel to
the patch to md5.c.
Comment 7 Andreas Jaeger 2012-08-15 18:57:41 UTC
Thanks for the bugreport, this is now fixed in current glibc sources.