[Audit] IsOnCurve method not called during unmarshal/new

[jpopxfile]

This is a suggestion only. Given that structure fields are only private on a package level in golang, this may not make sense to implement. In particular if consuming packages are free to modify the internal fields of the structure at any point, then IsOnCurve may need to be called to check the structure has not been altered into an invalid state.

The structure ECPoint represents a point on a given curve. Currently a method exists such that one can:

    point := NewECPoint(P256(), x, y)
    if !point.IsOnCurve() {
        // report error
    }

This relies on the programmer remembering to call IsOnCurve. We see such checks in the code:

https://github.com/binance-chain/tss-lib/blob/f90989ed35aa3f016ea35623c6faa697d9fb64a7/crypto/schnorr/schnorr_proof_test.go#L20

https://github.com/binance-chain/tss-lib/blob/f90989ed35aa3f016ea35623c6faa697d9fb64a7/crypto/mta/proofs.go#L173

https://github.com/binance-chain/tss-lib/blob/f90989ed35aa3f016ea35623c6faa697d9fb64a7/crypto/vss/feldman_vss_test.go#L36

https://github.com/binance-chain/tss-lib/blob/f90989ed35aa3f016ea35623c6faa697d9fb64a7/ecdsa/keygen/round_3.go#L127

but for defensive coding purposes we recommend not allowing the construction of an invalid curve point. This can be done by changing NewECPoint to validate and return an error:

func isOnCurve(c Curve, x, y *big.Int) bool {
  if x == nil || y == nil {
    return false
  }
  return c.IsOnCurve(x,y)
}

func (p *ECPoint) IsOnCurve() bool {
    return isOnCurve(p, p.coords[0], p.coords[1])
}

func NewECPoint(curve elliptic.Curve, X, Y *big.Int) *ECPoint, error {
  if !isOnCurve(curve, X, Y) {
        return nil, fmt.Errorf("Your error message here")
    }
    return &ECPoint{curve, [2]*big.Int{X, Y}}, nil
}

The unmarshal function UnmarshalJSON can likewise call isOnCurve to validate the point.