Skip to content
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

crypto/ecdsa: Sign() panics if public key is not set #70468

Open
guidovranken opened this issue Nov 20, 2024 · 5 comments
Open

crypto/ecdsa: Sign() panics if public key is not set #70468

guidovranken opened this issue Nov 20, 2024 · 5 comments
Assignees
Milestone

Comments

@guidovranken
Copy link

guidovranken commented Nov 20, 2024

Go version

go version devel go1.24-7c7170e Wed Nov 20 18:27:31 2024 +0000 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='clang-15'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++-15'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/jhg/.cache/go-build'
GODEBUG=''
GOENV='/home/jhg/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1928511716=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/jhg/oss-fuzz-379773077/p/go.mod'
GOMODCACHE='/home/jhg/oss-fuzz-379773077/go-dev/packages/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jhg/oss-fuzz-379773077/go-dev/packages'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/jhg/oss-fuzz-379773077/go-dev'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jhg/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/jhg/oss-fuzz-379773077/go-dev/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-7c7170e Wed Nov 20 18:27:31 2024 +0000'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

package main

import (
	"crypto/ecdsa"
	"crypto/elliptic"
	"crypto/rand"
	"crypto/sha256"
	"fmt"
	"math/big"
)

func main() {
	priv, ok := new(big.Int).SetString("123", 10)
	if ok == false {
		panic("Cannot decode bignum")
	}
	var priv_ecdsa ecdsa.PrivateKey
	priv_ecdsa.D = priv
	priv_ecdsa.PublicKey.Curve = elliptic.P256()

	msg := "hello, world"
	hash := sha256.Sum256([]byte(msg))

	r, s, err := ecdsa.Sign(rand.Reader, &priv_ecdsa, hash[:])
	fmt.Println(err)
	fmt.Println(r.String())
	fmt.Println(s.String())
}

What did you see happen?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4c2117]

goroutine 1 [running]:
math/big.(*Int).Sign(...)
	/home/jhg/oss-fuzz-379773077/go-dev/src/math/big/int.go:48
crypto/ecdsa.pointFromAffine({0x56bc88?, 0x646070?}, 0x0, 0x0)
	/home/jhg/oss-fuzz-379773077/go-dev/src/crypto/ecdsa/ecdsa.go:402 +0x37
crypto/ecdsa.privateKeyToFIPS[...](0xc0002d02c0, 0xc0002dff18)
	/home/jhg/oss-fuzz-379773077/go-dev/src/crypto/ecdsa/ecdsa.go:391 +0x38
crypto/ecdsa.signFIPS[...](0xc0002d02c0, 0xc0002c8df8, {0x56b020?, 0x66c280}, {0xc0002dfed8, 0x20, 0x20})
	/home/jhg/oss-fuzz-379773077/go-dev/src/crypto/ecdsa/ecdsa.go:234 +0x46
crypto/ecdsa.SignASN1({0x56b020, 0x66c280}, 0xc0002dff18, {0xc0002dfed8, 0x20, 0x20})
	/home/jhg/oss-fuzz-379773077/go-dev/src/crypto/ecdsa/ecdsa.go:220 +0x229
crypto/ecdsa.Sign({0x56b020?, 0x66c280?}, 0xbf7cbadf074d6483?, {0xc0002c8ed8?, 0xc0002c8ef8?, 0xc?})
	/home/jhg/oss-fuzz-379773077/go-dev/src/crypto/ecdsa/ecdsa_legacy.go:60 +0x37
main.main()
	/home/jhg/oss-fuzz-379773077/p/x.go:23 +0xf4

What did you expect to see?

No panic and output like:

<nil>
40753909606936490861524166827361514810969838335424734688996005945565630866707
3003946056421339230760555414094068582110502790139132375823642300394845145720
@guidovranken
Copy link
Author

This used to always work. Presumably broken due to 9776d02 or any of the related commits.

@ianlancetaylor
Copy link
Contributor

CC @golang/security

@FiloSottile
Copy link
Contributor

The joys of malleable key types. I am tempted to argue that such a key is corrupted, you can't just nil fields of a type arbitrarily, and worked by chance, but I guess we should avoid breakage if we can.

@FiloSottile FiloSottile self-assigned this Nov 20, 2024
@FiloSottile FiloSottile added this to the Go1.24 milestone Nov 20, 2024
@ianlancetaylor
Copy link
Contributor

This was caused by https://go.dev/cl/628677.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants