Re: sha512_process_bytes doesn't like odd sized buffers

[Jim Meyering]

"J. Scott Edwards" <address@hidden> wrote:
> I have been using the md5.c, sha1.c, sha256.c functions in a program
> of mine for some time.  I just started using the sha512.c functions in
> the same program and I have discovered that it gets unhappy if I send
> it buffers of 244 bytes, it works fine if I send it buffers of 256.  I
> believe I have found the bug,  it is in the line:
> "sha512_process_block (ctx->buffer, ctx->buflen & ~63, ctx);".  If I
> change the 63 to 127 it seems to work for 244 bytes as well.  I have
> included a patch below if that is useful:
>
> --- sha512.c.~1.1.~     2008-03-11 12:26:09.000000000 -0600
> +++ sha512.c    2008-03-15 02:46:55.000000000 -0600
> @@ -352,7 +352,7 @@
>
>        if (ctx->buflen > 128)
>         {
> -         sha512_process_block (ctx->buffer, ctx->buflen & ~63, ctx);
> +         sha512_process_block (ctx->buffer, ctx->buflen & ~127, ctx);
>
>           ctx->buflen &= 127;
>           /* The regions in the following copy operation cannot overlap.  */

Thank you for the report and patch!
The bug strikes whenever ctx->buflen|64 != 0.

It's not obvious, but I have confirmed that this bug does not
affect coreutils' sha512sum program, since its sole use of
sha512_process_bytes is via the sha512_stream function,
and that is immune, since in that code path, sha512_process_bytes'
incoming ctx->buflen is always 0, and the buggy code is never used.

Here's what I expect to push.
Please confirm the spelling of your name and email address
in the first patch, below.

The second patch removes the duplicate "ctx->buflen & ~127"
sub-expression in both gl/lib/sha256.c and gl/lib/sha512.c.
I've done the same to md5.c and sha1.c in gnulib.

>From 34e421bb4a729b26a19c560e9ac5e3f0e2ef8127 Mon Sep 17 00:00:00 2001
From: J. Scott Edwards <address@hidden>
Date: Sat, 15 Mar 2008 12:25:08 +0100
Subject: [PATCH] sha512_process_bytes: fix a bug in processing a buffer where 
length|64 != 0

* gl/lib/sha512.c (sha512_process_bytes): s/63/127/.

Signed-off-by: Jim Meyering <address@hidden>
---
 gl/lib/sha512.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gl/lib/sha512.c b/gl/lib/sha512.c
index e0109f8..4a41cb0 100644
--- a/gl/lib/sha512.c
+++ b/gl/lib/sha512.c
@@ -345,7 +345,7 @@ sha512_process_bytes (const void *buffer, size_t len, 
struct sha512_ctx *ctx)

       if (ctx->buflen > 128)
        {
-         sha512_process_block (ctx->buffer, ctx->buflen & ~63, ctx);
+         sha512_process_block (ctx->buffer, ctx->buflen & ~127, ctx);

          ctx->buflen &= 127;
          /* The regions in the following copy operation cannot overlap.  */
--
1.5.4.4.552.g3ab2


>From efb5aa0a723398e0672358f63e692d7575372c6f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 15 Mar 2008 12:32:55 +0100
Subject: [PATCH] sha512.c, sha256.c: Remove a redundant expression.

* gl/lib/sha512.c (sha512_process_bytes): AND-off the low bits in
"->buflen" only once, before calling sha512_process_block.
That's ok, since that function doesn't use the buflen member.
* gl/lib/sha256.c (sha256_process_bytes): Likewise.

Signed-off-by: Jim Meyering <address@hidden>
---
 gl/lib/sha256.c |    4 ++--
 gl/lib/sha512.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gl/lib/sha256.c b/gl/lib/sha256.c
index 4a632c9..fc3b52c 100644
--- a/gl/lib/sha256.c
+++ b/gl/lib/sha256.c
@@ -337,9 +337,9 @@ sha256_process_bytes (const void *buffer, size_t len, 
struct sha256_ctx *ctx)

       if (ctx->buflen > 64)
        {
-         sha256_process_block (ctx->buffer, ctx->buflen & ~63, ctx);
-
          ctx->buflen &= 63;
+         sha256_process_block (ctx->buffer, ctx->buflen, ctx);
+
          /* The regions in the following copy operation cannot overlap.  */
          memcpy (ctx->buffer,
                  &((char *) ctx->buffer)[(left_over + add) & ~63],
diff --git a/gl/lib/sha512.c b/gl/lib/sha512.c
index 4a41cb0..8714e8a 100644
--- a/gl/lib/sha512.c
+++ b/gl/lib/sha512.c
@@ -345,9 +345,9 @@ sha512_process_bytes (const void *buffer, size_t len, 
struct sha512_ctx *ctx)

       if (ctx->buflen > 128)
        {
-         sha512_process_block (ctx->buffer, ctx->buflen & ~127, ctx);
-
          ctx->buflen &= 127;
+         sha512_process_block (ctx->buffer, ctx->buflen, ctx);
+
          /* The regions in the following copy operation cannot overlap.  */
          memcpy (ctx->buffer,
                  &((char *) ctx->buffer)[(left_over + add) & ~127],
--
1.5.4.4.552.g3ab2