From 978e39e9e647d7359a41ac32992ef6ff5380be08 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Thu, 4 Nov 2021 16:08:08 -0400 Subject: [PATCH] crypto/elliptic: tolerate large inputs to IsOnCurve methods The behavior of all Curve methods and package functions when provided an off-curve point is undefined, except for IsOnCurve which should really always return false, not panic. Change-Id: I52f65df25c5af0314fef2c63d0778db72c0f1313 Reviewed-on: https://go-review.googlesource.com/c/go/+/361402 Trust: Filippo Valsorda Run-TryBot: Filippo Valsorda Reviewed-by: Roland Shoemaker TryBot-Result: Go Bot --- src/crypto/elliptic/elliptic_test.go | 10 ++++++++++ src/crypto/elliptic/p224.go | 4 ++++ src/crypto/elliptic/p521.go | 16 +++++++++++----- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/crypto/elliptic/elliptic_test.go b/src/crypto/elliptic/elliptic_test.go index c9744b5a51..d30a6939a4 100644 --- a/src/crypto/elliptic/elliptic_test.go +++ b/src/crypto/elliptic/elliptic_test.go @@ -241,6 +241,16 @@ func testMarshalCompressed(t *testing.T, curve Curve, x, y *big.Int, want []byte } } +func TestLargeIsOnCurve(t *testing.T) { + testAllCurves(t, func(t *testing.T, curve Curve) { + large := big.NewInt(1) + large.Lsh(large, 1000) + if curve.IsOnCurve(large, large) { + t.Errorf("(2^1000, 2^1000) is reported on the curve") + } + }) +} + func benchmarkAllCurves(t *testing.B, f func(*testing.B, Curve)) { tests := []struct { name string diff --git a/src/crypto/elliptic/p224.go b/src/crypto/elliptic/p224.go index 8f3622c89c..34079d14b1 100644 --- a/src/crypto/elliptic/p224.go +++ b/src/crypto/elliptic/p224.go @@ -50,6 +50,10 @@ func (curve p224Curve) Params() *CurveParams { } func (curve p224Curve) IsOnCurve(bigX, bigY *big.Int) bool { + if bigX.BitLen() > 224 || bigY.BitLen() > 224 { + return false + } + var x, y p224FieldElement p224FromBig(&x, bigX) p224FromBig(&y, bigY) diff --git a/src/crypto/elliptic/p521.go b/src/crypto/elliptic/p521.go index 4cc5f86d6d..e64007dfe3 100644 --- a/src/crypto/elliptic/p521.go +++ b/src/crypto/elliptic/p521.go @@ -55,19 +55,25 @@ func (curve p521Curve) Params() *CurveParams { } func (curve p521Curve) IsOnCurve(x, y *big.Int) bool { - // IsOnCurve is documented to reject (0, 0), so we don't use - // p521PointFromAffine, but let SetBytes reject the invalid Marshal output. - _, err := nistec.NewP521Point().SetBytes(Marshal(curve, x, y)) - return err == nil + // IsOnCurve is documented to reject (0, 0), the conventional point at + // infinity, which however is accepted by p521PointFromAffine. + if x.Sign() == 0 && y.Sign() == 0 { + return false + } + _, ok := p521PointFromAffine(x, y) + return ok } func p521PointFromAffine(x, y *big.Int) (p *nistec.P521Point, ok bool) { // (0, 0) is by convention the point at infinity, which can't be represented // in affine coordinates. Marshal incorrectly encodes it as an uncompressed - // point, which SetBytes correctly rejects. See Issue 37294. + // point, which SetBytes would correctly reject. See Issue 37294. if x.Sign() == 0 && y.Sign() == 0 { return nistec.NewP521Point(), true } + if x.BitLen() > 521 || y.BitLen() > 521 { + return nil, false + } p, err := nistec.NewP521Point().SetBytes(Marshal(P521(), x, y)) if err != nil { return nil, false -- 2.48.1