bcrypt: fix C compatibility code

The bcrypt implementation must append a zero byte to the user provided key to be compatible to C implementations. This will change the user provided key if the slice has enough capacity to hold the extra zero byte. This change always allocates a new slice for the C-compatible key. Fixes golang/go#20425 Change-Id: I8dc4e840c29711daabdabe58d83643cc0103cedd Reviewed-on: https://go-review.googlesource.com/43715 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
golang · May 22, 2017 · 6c586e1 · 6c586e1
1 parent 0fe9631
commit 6c586e1
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go
@@ -12,9 +12,10 @@ import (
 	"crypto/subtle"
 	"errors"
 	"fmt"
-	"golang.org/x/crypto/blowfish"
 	"io"
 	"strconv"
+
+	"golang.org/x/crypto/blowfish"
 )
 
 const (
@@ -205,15 +206,15 @@ func bcrypt(password []byte, cost int, salt []byte) ([]byte, error) {
 }
 
 func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cipher, error) {
-
 	csalt, err := base64Decode(salt)
 	if err != nil {
 		return nil, err
 	}
 
 	// Bug compatibility with C bcrypt implementations. They use the trailing
 	// NULL in the key string during expansion.
-	ckey := append(key, 0)
+	// We copy the key to prevent changing the underlying array.
+	ckey := append(key[:len(key):len(key)], 0)
 
 	c, err := blowfish.NewSaltedCipher(ckey, csalt)
 	if err != nil {

diff --git a/bcrypt/bcrypt_test.go b/bcrypt/bcrypt_test.go
@@ -224,3 +224,20 @@ func BenchmarkGeneration(b *testing.B) {
 		GenerateFromPassword(passwd, 10)
 	}
 }
+
+// See Issue https://github.com/golang/go/issues/20425.
+func TestNoSideEffectsFromCompare(t *testing.T) {
+	source := []byte("passw0rd123456")
+	password := source[:len(source)-6]
+	token := source[len(source)-6:]
+	want := make([]byte, len(source))
+	copy(want, source)
+
+	wantHash := []byte("$2a$10$LK9XRuhNxHHCvjX3tdkRKei1QiCDUKrJRhZv7WWZPuQGRUM92rOUa")
+	_ = CompareHashAndPassword(wantHash, password)
+
+	got := bytes.Join([][]byte{password, token}, []byte(""))
+	if !bytes.Equal(got, want) {
+		t.Errorf("got=%q want=%q", got, want)
+	}
+}