golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.21k stars 17.7k forks source link

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

Open guidovranken opened 1 day ago

guidovranken commented 1 day ago

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
gabyhelp commented 1 day ago

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

guidovranken commented 1 day ago

This used to always work. Presumably broken due to https://github.com/golang/go/commit/9776d028f4b99b9a935dae9f63f32871b77c49af or any of the related commits.

ianlancetaylor commented 1 day ago

CC @golang/security

FiloSottile commented 1 day ago

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.

ianlancetaylor commented 1 day ago

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