-
Notifications
You must be signed in to change notification settings - Fork 83
Open
Description
Instructions
I created a minimal reproduction here using flow-go-sdk v1.3.3: https://github.com/sul3n3t/flow-panic
Problem
While updating to go 1.24, some existing code triggered a panic during signing. The panic is caused because stdlib crypto code expects fields of the public component of a private key to be present.
Steps to Reproduce
package flowpanic_test
import (
"testing"
"github.com/onflow/flow-go-sdk/crypto"
)
func TestSign(t *testing.T) {
privateKey, err := crypto.GeneratePrivateKey(crypto.ECDSA_P256, make([]byte, 32))
if err != nil {
t.Fatal(err)
}
// privateKey.PublicKey() // necessary on go 1.24 to avoid panic
signer, err := crypto.NewInMemorySigner(privateKey, crypto.SHA3_256)
if err != nil {
t.Fatal(err)
}
_, err = signer.Sign([]byte("some message"))
if err != nil {
t.Fatal(err)
}
}
Results in
--- FAIL: TestSign (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x10 pc=0x1045aab08]
goroutine 36 [running]:
testing.tRunner.func1.2({0x1047afac0, 0x104958b50})
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/testing/testing.go:1734 +0x1ac
testing.tRunner.func1()
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/testing/testing.go:1737 +0x334
panic({0x1047afac0?, 0x104958b50?})
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/runtime/panic.go:787 +0x124
math/big.(*Int).Sign(...)
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/math/big/int.go:48
crypto/ecdsa.pointFromAffine({0x1047f9910?, 0x104959080?}, 0x0, 0x0)
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/crypto/ecdsa/ecdsa.go:416 +0x38
crypto/ecdsa.privateKeyToFIPS[...](0x140001a6e00, 0x1400019d440)
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/crypto/ecdsa/ecdsa.go:405 +0x3c
crypto/ecdsa.signFIPS[...](0x140001a6e00, 0x14000064cf8, {0x1047f7960?, 0x14000198160}, {0x140001b4480, 0x20, 0x20})
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/crypto/ecdsa/ecdsa.go:244 +0x70
crypto/ecdsa.SignASN1({0x1047f7960, 0x14000198160}, 0x1400019d440, {0x140001b4480, 0x20, 0x20})
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/crypto/ecdsa/ecdsa.go:227 +0x234
crypto/ecdsa.Sign({0x1047f7960?, 0x14000198160?}, 0x1?, {0x140001b4480?, 0x14000064e28?, 0x1045bafa8?})
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/crypto/ecdsa/ecdsa_legacy.go:65 +0x2c
github.com/onflow/crypto.(*prKeyECDSA).signHash(0x140002001e0, {0x140001b4480?, 0x14000064ea8?, 0x104487d7c?})
/Users/justin/go/pkg/mod/github.com/onflow/crypto@v0.25.1/ecdsa.go:80 +0x54
github.com/onflow/crypto.(*prKeyECDSA).Sign(0x140002001e0, {0x140001f4660, 0xc, 0xc}, {0x1047f9880, 0x14000216000})
/Users/justin/go/pkg/mod/github.com/onflow/crypto@v0.25.1/ecdsa.go:118 +0x1c0
github.com/onflow/flow-go-sdk/crypto.InMemorySigner.Sign(...)
/Users/justin/go/pkg/mod/github.com/onflow/flow-go-sdk@v1.3.3/crypto/crypto.go:133
github.com/sul3n3t/flow-panic_test.TestSign(0x14000184c40)
/Users/justin/go/src/flow-panic/sign_test.go:20 +0x124
testing.tRunner(0x14000184c40, 0x1047f4bc8)
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/testing/testing.go:1792 +0xe4
created by testing.(*T).Run in goroutine 1
/Users/justin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/src/testing/testing.go:1851 +0x374
FAIL github.com/sul3n3t/flow-panic 0.209s
Acceptance Criteria
The test is expected to pass without calling the .PublicKey() method
Context
I came across this while updating go in a much larger repo. I have a workaround by calling crypto.PrivateKey#PublicKey().
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
No status