New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/crypto/bcrypt: bcrypt.expensiveBlowfishSetup changes slice's underlying array #20425
Labels
Milestone
Comments
I think this is unintended behavior and should be fixed. Thanks for the bug report @magisterquis. |
CL https://golang.org/cl/43715 mentions this issue. |
c-expert-zigbee
pushed a commit
to c-expert-zigbee/crypto_go
that referenced
this issue
Mar 28, 2022
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 <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
c-expert-zigbee
pushed a commit
to c-expert-zigbee/crypto_go
that referenced
this issue
Mar 29, 2022
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 <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LewiGoddard
pushed a commit
to LewiGoddard/crypto
that referenced
this issue
Feb 16, 2023
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 <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
BiiChris
pushed a commit
to BiiChris/crypto
that referenced
this issue
Sep 15, 2023
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 <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
Output:
What did you expect to see?
I expected the value of
token
to be unchanged after the call tobcrypt.CompareAndHashPassword
.What did you see instead?
The first byte of
token
was changed. This seems to happen in the call tobcrypt.expensiveBlowfishSetup
when it appends a null byte topassword
to be compatible with C implementations.https://github.com/golang/crypto/blob/master/bcrypt/bcrypt.go#L216
Unfortunately this changes the slice's underlying array, which can have knock-on effects if other slices refer to the same array (as in my 2FA-derived example).
The text was updated successfully, but these errors were encountered: