Protect jws.Verify() and jwe.Encrypt() from panic on go1.19+ (#841)

* Protect jws.Verify() from panic on go1.19+ * Same problem, but in jwe
lestrrat-go · Nov 18, 2022 · fd33b62 · fd33b62
1 parent a6e06cd
commit fd33b62
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 0 deletions.
diff --git a/jwe/internal/keygen/keygen.go b/jwe/internal/keygen/keygen.go
@@ -88,6 +88,9 @@ func (g Ecdhes) Generate() (ByteSource, error) {
 	pubinfo := make([]byte, 4)
 	binary.BigEndian.PutUint32(pubinfo, uint32(g.keysize)*8)
 
+	if !priv.PublicKey.Curve.IsOnCurve(g.pubkey.X, g.pubkey.Y) {
+		return nil, fmt.Errorf(`public key used does not contain a point (X,Y) on the curve`)
+	}
 	z, _ := priv.PublicKey.Curve.ScalarMult(g.pubkey.X, g.pubkey.Y, priv.D.Bytes())
 	zBytes := ecutil.AllocECPointBuffer(z, priv.PublicKey.Curve)
 	defer ecutil.ReleaseECPointBuffer(zBytes)

diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go
@@ -819,3 +819,25 @@ func TestGH803(t *testing.T) {
 	require.Equal(t, apu, msg.ProtectedHeaders().AgreementPartyUInfo())
 	require.Equal(t, apv, msg.ProtectedHeaders().AgreementPartyVInfo())
 }
+
+func TestGH840(t *testing.T) {
+	// Go 1.19+ panics if elliptic curve operations are called against
+	// a point that's _NOT_ on the curve
+	untrustedJWK := []byte(`{
+		"kty": "EC",
+		"crv": "P-256",
+		"x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqx7D4",
+		"y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM",
+		"d": "870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE"
+	}`)
+
+	privkey, err := jwk.ParseKey(untrustedJWK)
+	require.NoError(t, err, `jwk.ParseKey should succeed`)
+
+	pubkey, err := privkey.PublicKey()
+	require.NoError(t, err, `privkey.PublicKey should succeed`)
+
+	const payload = `Lorem ipsum`
+	_, err = jwe.Encrypt([]byte(payload), jwe.WithKey(jwa.ECDH_ES_A128KW, pubkey))
+	require.Error(t, err, `jwe.Encrypt should fail (instead of panic)`)
+}
diff --git a/jws/ecdsa.go b/jws/ecdsa.go
@@ -172,6 +172,10 @@ func (v *ecdsaVerifier) Verify(payload []byte, signature []byte, key interface{}
 		}
 	}
 
+	if !pubkey.Curve.IsOnCurve(pubkey.X, pubkey.Y) {
+		return fmt.Errorf(`public key used does not contain a point (X,Y) on the curve`)
+	}
+
 	r := pool.GetBigInt()
 	s := pool.GetBigInt()
 	defer pool.ReleaseBigInt(r)

diff --git a/jws/jws_test.go b/jws/jws_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/lestrrat-go/jwx/v2/jws"
+	"github.com/lestrrat-go/jwx/v2/jwt"
 	"github.com/lestrrat-go/jwx/v2/x25519"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -1919,3 +1920,34 @@ func TestGH681(t *testing.T) {
 		return
 	}
 }
+
+func TestGH840(t *testing.T) {
+	// Go 1.19+ panics if elliptic curve operations are called against
+	// a point that's _NOT_ on the curve
+	untrustedJWK := []byte(`{
+		"kty": "EC",
+		"crv": "P-256",
+		"x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqx7D4",
+		"y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM",
+		"d": "870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE"
+	}`)
+
+	// Parse, serialize, slice and dice JWKs!
+	privkey, err := jwk.ParseKey(untrustedJWK)
+	require.NoError(t, err, `jwk.ParseKey should succeed`)
+
+	pubkey, err := jwk.PublicKeyOf(privkey)
+	require.NoError(t, err, `jwk.PublicKeyOf should succeed`)
+
+	tok, err := jwt.NewBuilder().
+		Issuer(`github.com/lestrrat-go/jwx`).
+		IssuedAt(time.Now()).
+		Build()
+	require.NoError(t, err, `jwt.NewBuilder should succeed`)
+
+	signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES256, privkey))
+	require.NoError(t, err, `jwt.Sign should succeed`)
+
+	_, err = jwt.Parse(signed, jwt.WithKey(jwa.ES256, pubkey))
+	require.Error(t, err, `jwt.Parse should FAIL`) // pubkey's X/Y is not on the curve
+}