Browse Source

sha3: fix bug in cSHAKE Clone()

Clone() made a copy of the Keccak state after invoking clone(), which is not
supported, since the "buf" slice in the Keccak state must point to the "storage"
array, and if the state is copied directly it will keep pointing to the storage
returned by clone().

Fix it by embedding a pointer to the Keccak state instead of the state itself.

Change-Id: I7d392963ec65d784a360f6c12a7935a9a9a788b5
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/173018
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Conrado P. L. Gouvea 6 years ago
parent
commit
22d7a77e9e
2 changed files with 17 additions and 14 deletions
  1. 14 11
      sha3/sha3_test.go
  2. 3 3
      sha3/shake.go

+ 14 - 11
sha3/sha3_test.go

@@ -338,22 +338,25 @@ func TestReset(t *testing.T) {
 func TestClone(t *testing.T) {
 	out1 := make([]byte, 16)
 	out2 := make([]byte, 16)
-	in := sequentialBytes(0x100)
 
-	for _, v := range testShakes {
-		h1 := v.constructor(nil, []byte{0x01})
-		h1.Write([]byte{0x01})
+	// Test for sizes smaller and larger than block size.
+	for _, size := range []int{0x1, 0x100} {
+		in := sequentialBytes(size)
+		for _, v := range testShakes {
+			h1 := v.constructor(nil, []byte{0x01})
+			h1.Write([]byte{0x01})
 
-		h2 := h1.Clone()
+			h2 := h1.Clone()
 
-		h1.Write(in)
-		h1.Read(out1)
+			h1.Write(in)
+			h1.Read(out1)
 
-		h2.Write(in)
-		h2.Read(out2)
+			h2.Write(in)
+			h2.Read(out2)
 
-		if !bytes.Equal(out1, out2) {
-			t.Error("\nExpected:\n", hex.EncodeToString(out1), "\ngot:\n", hex.EncodeToString(out2))
+			if !bytes.Equal(out1, out2) {
+				t.Error("\nExpected:\n", hex.EncodeToString(out1), "\ngot:\n", hex.EncodeToString(out2))
+			}
 		}
 	}
 }

+ 3 - 3
sha3/shake.go

@@ -41,7 +41,7 @@ type ShakeHash interface {
 
 // cSHAKE specific context
 type cshakeState struct {
-	state // SHA-3 state context and Read/Write operations
+	*state // SHA-3 state context and Read/Write operations
 
 	// initBlock is the cSHAKE specific initialization set of bytes. It is initialized
 	// by newCShake function and stores concatenation of N followed by S, encoded
@@ -82,7 +82,7 @@ func leftEncode(value uint64) []byte {
 }
 
 func newCShake(N, S []byte, rate int, dsbyte byte) ShakeHash {
-	c := cshakeState{state: state{rate: rate, dsbyte: dsbyte}}
+	c := cshakeState{state: &state{rate: rate, dsbyte: dsbyte}}
 
 	// leftEncode returns max 9 bytes
 	c.initBlock = make([]byte, 0, 9*2+len(N)+len(S))
@@ -104,7 +104,7 @@ func (c *cshakeState) Reset() {
 func (c *cshakeState) Clone() ShakeHash {
 	b := make([]byte, len(c.initBlock))
 	copy(b, c.initBlock)
-	return &cshakeState{state: *c.clone(), initBlock: b}
+	return &cshakeState{state: c.clone(), initBlock: b}
 }
 
 // Clone returns copy of SHAKE context within its current state.