diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go index f8b807f9c3..202fa8aff4 100644 --- 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,7 +206,6 @@ 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 @@ -213,7 +213,8 @@ func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cip // 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 index f08a6f5b22..aecf759eb1 100644 --- 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) + } +}