crypto/md5: fix casting of d.nx in UnmarshalBinary

Fixes #29545

Change-Id: Ida98c23b8fc5c676d8bf0b3daad8320e495ebf64
GitHub-Last-Rev: d38e8a90c75f92031f6a8cf1f69f7bc7c28a52d8
GitHub-Pull-Request: golang/go#29546
Reviewed-on: https://go-review.googlesource.com/c/156297
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/crypto/md5/md5.go b/src/crypto/md5/md5.go
index 3e66db6d0d7f169aa27ad7ef1a306e00211770bf..011578404751aca16699dfe73949247798564723 100644 (file)
--- a/src/crypto/md5/md5.go
+++ b/src/crypto/md5/md5.go
@@ -83,7 +83,7 @@ func (d *digest) UnmarshalBinary(b []byte) error {
        b, d.s[3] = consumeUint32(b)
        b = b[copy(d.x[:], b):]
        b, d.len = consumeUint64(b)
-       d.nx = int(d.len) % BlockSize
+       d.nx = int(d.len % BlockSize)
        return nil
 }
 

diff --git a/src/crypto/md5/md5_test.go b/src/crypto/md5/md5_test.go
index 64a62e4730587eeeb4c7307ead150aa9782c091d..34c7f541c5740ca95492bc87fabc05abdb71f462 100644 (file)
--- a/src/crypto/md5/md5_test.go
+++ b/src/crypto/md5/md5_test.go
@@ -9,6 +9,7 @@ import (
        "crypto/rand"
        "encoding"
        "fmt"
+       "hash"
        "io"
        "testing"
        "unsafe"
@@ -153,6 +154,63 @@ func TestBlockGeneric(t *testing.T) {
        }
 }
 
+// Tests for unmarshaling hashes that have hashed a large amount of data
+// The initial hash generation is omitted from the test, because it takes a long time.
+// The test contains some already-generated states, and their expected sums
+// Tests a problem that is outlined in Github issue #29541
+// The problem is triggered when an amount of data has been hashed for which
+// the data length has a 1 in the 32nd bit. When casted to int, this changes
+// the sign of the value, and causes the modulus operation to return a
+// different result.
+type unmarshalTest struct {
+       state string
+       sum   string
+}
+
+var largeUnmarshalTests = []unmarshalTest{
+       // Data length: 7_102_415_735
+       unmarshalTest{
+               state: "md5\x01\xa5\xf7\xf0=\xd6S\x85\xd9M\n}\xc3\u0601\x89\xe7@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuv\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xa7VCw",
+               sum:   "cddefcf74ffec709a0b45a6a987564d5",
+       },
+       // Data length: 6_565_544_823
+       unmarshalTest{
+               state: "md5\x01{\xda\x1a\xc7\xc9'?\x83EX\xe0\x88q\xfeG\x18@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuv\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x87VCw",
+               sum:   "fd9f41874ab240698e7bc9c3ae70c8e4",
+       },
+}
+
+func safeSum(h hash.Hash) (sum []byte, err error) {
+       defer func() {
+               if r := recover(); r != nil {
+                       err = fmt.Errorf("sum panic: %v", r)
+               }
+       }()
+
+       return h.Sum(nil), nil
+}
+
+func TestLargeHashes(t *testing.T) {
+       for i, test := range largeUnmarshalTests {
+
+               h := New()
+               if err := h.(encoding.BinaryUnmarshaler).UnmarshalBinary([]byte(test.state)); err != nil {
+                       t.Errorf("test %d could not unmarshal: %v", i, err)
+                       continue
+               }
+
+               sum, err := safeSum(h)
+               if err != nil {
+                       t.Errorf("test %d could not sum: %v", i, err)
+                       continue
+               }
+
+               if fmt.Sprintf("%x", sum) != test.sum {
+                       t.Errorf("test %d sum mismatch: expect %s got %x", i, test.sum, sum)
+               }
+       }
+}
+
 var bench = New()
 var buf = make([]byte, 8192+1)
 var sum = make([]byte, bench.Size())