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 <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>

diff --git a/src/crypto/elliptic/elliptic_test.go b/src/crypto/elliptic/elliptic_test.go
index c9744b5a51c8ca1d3b674922fbfbc078ca8d0b06..d30a6939a4e35c5382d15c52d78b03de2a3cf884 100644 (file)
--- 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 8f3622c89cf2ab1fcae88dcdcb24d497d0902f5c..34079d14b12496c7ba375e7a1452ff0b67d1816e 100644 (file)
--- 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 4cc5f86d6dd9509b29e0e26f458002a4667df9c3..e64007dfe35aad3a17906a054953a6d68280be88 100644 (file)
--- 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