Skip to content

crypto/sha256: on 32-bit platforms, panic trying to Sum after a large amount of data hashed #29517

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

Closed
jblebrun opened this issue Jan 3, 2019 · 1 comment
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@jblebrun
Copy link
Contributor

jblebrun commented Jan 3, 2019

What version of Go are you using (go version)?

$ go version 1.11.1

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env
$ go env

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.1/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w6/9bjdntr539z1v138l_8mvmqshs5f5p/T/go-build219458045=/tmp/go-build -gno-record-gcc-switches -fno-common"

Note: the issue only occurs targeting architectures for which an int is 32 bits. I can easily trigger the problem by building with GOARCH=386.

What did you do?

Hash a large amount of data (length large than a platform int), save the hash using binary.Marshal, restore it using binary.Unmarshal, and then try to Sum it.

This play.golang.org link contains a simple example that triggers the issue:
https://play.golang.org/p/P9o_9GsdUf0

When I run it there, the arch I see is amd64p32, so the issue occurs.

What did you expect to see?

I expect to be able to successfully get a checksum for the unmarshaled sha256 without panic.

What did you see instead?

Upon calling h.Sum(nil), I get a panic similar to:

panic: d.nx != 0

goroutine 1 [running]:
crypto/sha256.(*digest).checkSum(0x114306fc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.11.1/libexec/src/crypto/sha256/sha256.go:253 +0x2d7
crypto/sha256.(*digest).Sum(0x1146e000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.11.1/libexec/src/crypto/sha256/sha256.go:229 +0x36
main.main()
        /test.go:26 +0xe5
exit status 2

Possible resolution

I can resolve this issue by copying the sha256 code into my own repository, and fixing line 103 to cast d.nx to an int after modding d.len by chunk.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/156118 mentions this issue: crypto/sha256: fix casting of d.nx in UnmarshalBinary

@bradfitz bradfitz changed the title On 32-bit platforms, trying to sum an unmarshaled sha256 hash can cause panic after a large amount of data has been hashed. crypto/sha256: on 32-bit platforms, panic trying to Sum after a large amount of data hashed Jan 3, 2019
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 3, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Jan 3, 2019
jblebrun added a commit to jblebrun/go that referenced this issue Jan 3, 2019
@golang golang locked and limited conversation to collaborators Jan 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants