From 17db262f9b0f2b0d48075e380a440231d9385762 Mon Sep 17 00:00:00 2001 From: enzo Date: Fri, 10 Apr 2026 09:03:31 +0000 Subject: [PATCH 1/2] Validate curve points in PublicKeyFromBytes [PAY-2108] Add IsOnCurve check to reject invalid elliptic curve points at parse time, preventing crypto/elliptic.ScalarMult panics in Go 1.24+. Co-Authored-By: Claude Opus 4.6 (1M context) --- encryption/ecies/publickey.go | 10 +++++-- encryption/ecies/publickey_test.go | 44 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/encryption/ecies/publickey.go b/encryption/ecies/publickey.go index b97d75f..26e7b3d 100644 --- a/encryption/ecies/publickey.go +++ b/encryption/ecies/publickey.go @@ -36,11 +36,15 @@ func PublicKeyFromBytes(b []byte, curve elliptic.Curve) (*PublicKey, error) { return nil, fmt.Errorf("invalid key length") } + x := new(big.Int).SetBytes(b[1 : size+1]) + y := new(big.Int).SetBytes(b[size+1:]) + if !curve.IsOnCurve(x, y) { + return nil, fmt.Errorf("point is not on the curve") + } + return &PublicKey{ curve: curve, - Point: &Point{ - X: new(big.Int).SetBytes(b[1 : size+1]), - Y: new(big.Int).SetBytes(b[size+1:])}, + Point: &Point{X: x, Y: y}, }, nil } diff --git a/encryption/ecies/publickey_test.go b/encryption/ecies/publickey_test.go index e1988f5..bf781fa 100644 --- a/encryption/ecies/publickey_test.go +++ b/encryption/ecies/publickey_test.go @@ -1,6 +1,10 @@ package ecies_test import ( + "crypto/elliptic" + "encoding/base64" + "encoding/hex" + "math/big" "testing" "github.com/stretchr/testify/assert" @@ -61,3 +65,43 @@ func Test_PublicKeyFromHex_Error(t *testing.T) { assert.Contains(err.Error(), "invalid key length") assert.Nil(pub) } + +// invalidP521Bytes returns uncompressed P521 public key bytes (0x04 || X || Y) +// with X=1, Y=1 which is not on the P521 curve. +func invalidP521Bytes() []byte { + size := (elliptic.P521().Params().BitSize + 7) / 8 // 66 + b := make([]byte, 1+2*size) + b[0] = 0x04 + big.NewInt(1).FillBytes(b[1 : size+1]) + big.NewInt(1).FillBytes(b[size+1:]) + return b +} + +func Test_PublicKeyFromBytes_InvalidCurvePoint(t *testing.T) { + assert := assert.New(t) + + pub, err := ecies.PublicKeyFromBytes(invalidP521Bytes(), elliptic.P521()) + assert.Error(err) + assert.Contains(err.Error(), "point is not on the curve") + assert.Nil(pub) +} + +func Test_PublicKeyFromBase64_InvalidCurvePoint(t *testing.T) { + assert := assert.New(t) + + b64 := base64.StdEncoding.EncodeToString(invalidP521Bytes()) + pub, err := ecies.PublicKeyFromBase64(b64, elliptic.P521()) + assert.Error(err) + assert.Contains(err.Error(), "point is not on the curve") + assert.Nil(pub) +} + +func Test_PublicKeyFromHex_InvalidCurvePoint(t *testing.T) { + assert := assert.New(t) + + h := hex.EncodeToString(invalidP521Bytes()) + pub, err := ecies.PublicKeyFromHex(h, elliptic.P521()) + assert.Error(err) + assert.Contains(err.Error(), "point is not on the curve") + assert.Nil(pub) +} From 539c0291107a36e692e9e6f89bb0e628ec8ac045 Mon Sep 17 00:00:00 2001 From: enzo Date: Fri, 10 Apr 2026 09:10:10 +0000 Subject: [PATCH 2/2] Add reviewer comments to curve point validation [PAY-2108] Add explanatory comments to the IsOnCurve check and test helpers to clarify the Go 1.24+ ScalarMult panic context for reviewers. Co-Authored-By: Claude Opus 4.6 (1M context) --- encryption/ecies/publickey.go | 5 +++++ encryption/ecies/publickey_test.go | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/encryption/ecies/publickey.go b/encryption/ecies/publickey.go index 26e7b3d..a6ce3c1 100644 --- a/encryption/ecies/publickey.go +++ b/encryption/ecies/publickey.go @@ -38,6 +38,11 @@ func PublicKeyFromBytes(b []byte, curve elliptic.Curve) (*PublicKey, error) { x := new(big.Int).SetBytes(b[1 : size+1]) y := new(big.Int).SetBytes(b[size+1:]) + + // Reject points not on the curve to prevent crypto/elliptic.ScalarMult panics + // introduced in Go 1.24+. Without this check, corrupted or malicious payloads + // with valid-length but off-curve coordinates cause a panic in ECDH computation. + // See PAY-2108. if !curve.IsOnCurve(x, y) { return nil, fmt.Errorf("point is not on the curve") } diff --git a/encryption/ecies/publickey_test.go b/encryption/ecies/publickey_test.go index bf781fa..2764e17 100644 --- a/encryption/ecies/publickey_test.go +++ b/encryption/ecies/publickey_test.go @@ -66,17 +66,22 @@ func Test_PublicKeyFromHex_Error(t *testing.T) { assert.Nil(pub) } -// invalidP521Bytes returns uncompressed P521 public key bytes (0x04 || X || Y) -// with X=1, Y=1 which is not on the P521 curve. +// invalidP521Bytes builds a well-formed uncompressed public key (0x04 || X || Y) +// whose coordinates (1, 1) are NOT on the P521 curve. +// This simulates corrupted or malicious encrypted payloads where the embedded +// ephemeral public key has valid length but invalid curve coordinates. +// Go 1.24+ panics in crypto/elliptic.ScalarMult for such points (PAY-2108). func invalidP521Bytes() []byte { - size := (elliptic.P521().Params().BitSize + 7) / 8 // 66 + size := (elliptic.P521().Params().BitSize + 7) / 8 // 66 bytes b := make([]byte, 1+2*size) - b[0] = 0x04 + b[0] = 0x04 // uncompressed point prefix big.NewInt(1).FillBytes(b[1 : size+1]) big.NewInt(1).FillBytes(b[size+1:]) return b } +// Test_PublicKeyFromBytes_InvalidCurvePoint verifies that off-curve points are +// rejected at parse time rather than causing a panic in downstream ScalarMult. func Test_PublicKeyFromBytes_InvalidCurvePoint(t *testing.T) { assert := assert.New(t) @@ -86,6 +91,8 @@ func Test_PublicKeyFromBytes_InvalidCurvePoint(t *testing.T) { assert.Nil(pub) } +// Test_PublicKeyFromBase64_InvalidCurvePoint ensures the validation propagates +// through the base64 parsing path (PublicKeyFromBase64 -> PublicKeyFromBytes). func Test_PublicKeyFromBase64_InvalidCurvePoint(t *testing.T) { assert := assert.New(t) @@ -96,6 +103,8 @@ func Test_PublicKeyFromBase64_InvalidCurvePoint(t *testing.T) { assert.Nil(pub) } +// Test_PublicKeyFromHex_InvalidCurvePoint ensures the validation propagates +// through the hex parsing path (PublicKeyFromHex -> PublicKeyFromBytes). func Test_PublicKeyFromHex_InvalidCurvePoint(t *testing.T) { assert := assert.New(t)