diff --git a/driver/amcl/fp256bn.go b/driver/amcl/fp256bn.go index bd8df82..329eda3 100644 --- a/driver/amcl/fp256bn.go +++ b/driver/amcl/fp256bn.go @@ -177,12 +177,16 @@ func (p *Fp256bn) HashToG1WithDomain(data, domain []byte) driver.G1 { return &fp256bnG1{*FP256BN.Bls_hash(string(mac.Sum(nil)))} } +// HashToG2 always panics: FP256BN has no supported hash-to-G2 map. Use a curve +// with native G2 hashing support (e.g. BLS12-381, BN254) if G2 hashing is required. func (p *Fp256bn) HashToG2(data []byte) driver.G2 { - panic("HashToG2 is not available for this curve") + panic("driver/amcl: HashToG2 is not implemented for FP256BN — use a curve with native G2 hashing support (e.g. BLS12-381, BN254)") } +// HashToG2WithDomain always panics: FP256BN has no supported hash-to-G2 map. Use a curve +// with native G2 hashing support (e.g. BLS12-381, BN254) if G2 hashing is required. func (p *Fp256bn) HashToG2WithDomain(data, domain []byte) driver.G2 { - panic("HashToG2WithDomain is not available for this curve") + panic("driver/amcl: HashToG2WithDomain is not implemented for FP256BN — use a curve with native G2 hashing support (e.g. BLS12-381, BN254)") } /*********************************************************************/ diff --git a/driver/amcl/fp256bn_miracl.go b/driver/amcl/fp256bn_miracl.go index 2e4a517..1972e81 100644 --- a/driver/amcl/fp256bn_miracl.go +++ b/driver/amcl/fp256bn_miracl.go @@ -178,12 +178,16 @@ func (p *Fp256Miraclbn) HashToG1WithDomain(data, domain []byte) driver.G1 { return &fp256bnMiraclG1{*bls_hash_to_point_miracl(data, domain)} } +// HashToG2 always panics: FP256BN has no supported hash-to-G2 map. Use a curve +// with native G2 hashing support (e.g. BLS12-381, BN254) if G2 hashing is required. func (p *Fp256Miraclbn) HashToG2(data []byte) driver.G2 { - panic("HashToG2 is not available for this curve") + panic("driver/amcl: HashToG2 is not implemented for FP256BN — use a curve with native G2 hashing support (e.g. BLS12-381, BN254)") } +// HashToG2WithDomain always panics: FP256BN has no supported hash-to-G2 map. Use a curve +// with native G2 hashing support (e.g. BLS12-381, BN254) if G2 hashing is required. func (p *Fp256Miraclbn) HashToG2WithDomain(data, domain []byte) driver.G2 { - panic("HashToG2WithDomain is not available for this curve") + panic("driver/amcl: HashToG2WithDomain is not implemented for FP256BN — use a curve with native G2 hashing support (e.g. BLS12-381, BN254)") } /*********************************************************************/ diff --git a/driver/gurvy/bls12381/bls12-381.go b/driver/gurvy/bls12381/bls12-381.go index b2b0b93..4aa385a 100644 --- a/driver/gurvy/bls12381/bls12-381.go +++ b/driver/gurvy/bls12381/bls12-381.go @@ -46,6 +46,16 @@ func init() { // The rawBigInt field is non-nil only for special values like GroupOrder // (which equals p and is 0 in the field but needs its actual big.Int value // for operations like Mod and InvModP). +// +// Only methods that explicitly branch on rawBigInt (IsZero, IsOne, Bytes, Equals, +// Copy, Clone, String, Mod, InvModP, InvModOrder, PowMod, toBigInt) preserve the +// true big-int value. Plus/Minus/Mul operate on val directly (val == 0 whenever +// rawBigInt != nil, since p mod p == 0), so arithmetic combining a rawBigInt-backed +// Zr with another Zr via Plus/Minus/Mul silently ignores the raw value. This is safe +// today because GroupOrder is only ever used as a PowMod exponent (where the result +// is invariant to the field reduction by Fermat's little theorem) or passed to the +// rawBigInt-aware methods above — do not add a new arithmetic use of GroupOrder via +// Plus/Minus/Mul without accounting for this. type Zr struct { val fr.Element rawBigInt *big.Int // non-nil only for GroupOrder @@ -246,6 +256,11 @@ func (g *G1) Mul(a driver.Zr) driver.G1 { return gc } +// Mul2 computes [e]g + [f]Q via a joint Strauss-Shamir scalar multiplication. +// Benchmarked against two independent Mul calls plus an Add: allocates far less +// (1 vs ~26 allocs) but is not faster in wall-clock time, because — unlike Mul — +// it does not use the GLV endomorphism speedup, so it forgoes the ~2x speedup that +// GLV gives each individual scalar multiplication. func (g *G1) Mul2(e driver.Zr, Q driver.G1, f driver.Zr) driver.G1 { bi1 := bigIntPool.Get() defer bigIntPool.Put(bi1) @@ -867,7 +882,9 @@ func (c *BBSCurve) HashToG2WithDomain(data, domain []byte) driver.G2 { } // JointScalarMultiplication computes [s1]a1+[s2]a2 using Strauss-Shamir technique -// where a1 and a2 are affine points. +// where a1 and a2 are affine points. This does not use the GLV endomorphism (unlike +// G1Jac.ScalarMultiplication), so it is an allocation optimization over two independent +// scalar multiplications plus an addition, not a wall-clock optimization. func JointScalarMultiplication(p *bls12381.G1Jac, a1, a2 *bls12381.G1Affine, s1, s2 *big.Int) *bls12381.G1Jac { var res, p1, p2 bls12381.G1Jac res.Set(&g1Infinity) diff --git a/fuzz_test.go b/fuzz_test.go new file mode 100644 index 0000000..fcaf948 --- /dev/null +++ b/fuzz_test.go @@ -0,0 +1,63 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package math + +import ( + "fmt" + "testing" +) + +// FuzzUnmarshalJSONCurveID targets the C1 fix: an out-of-range or negative +// "curve" field in the JSON payload must be rejected with an error, never +// cause an index-out-of-range panic when indexing Curves. +func FuzzUnmarshalJSONCurveID(f *testing.F) { + f.Add(0, []byte("AAAA")) + f.Add(-1, []byte("AAAA")) + f.Add(len(Curves), []byte("AAAA")) + f.Add(len(Curves)+1000, []byte("AAAA")) + f.Add(1<<31-1, []byte("")) + f.Add(-1<<31, []byte("!!!!")) + + f.Fuzz(func(t *testing.T, curveID int, element []byte) { + payload := fmt.Appendf(nil, `{"curve": %d, "element": %q}`, curveID, element) + + zr := &Zr{} + _ = zr.UnmarshalJSON(payload) + + g1 := &G1{} + _ = g1.UnmarshalJSON(payload) + + g2 := &G2{} + _ = g2.UnmarshalJSON(payload) + + gt := &Gt{} + _ = gt.UnmarshalJSON(payload) + }) +} + +// FuzzHashToGWithDomain targets the C2 fix: HashToG1WithDomain/HashToG2WithDomain +// must never panic regardless of domain length or content, returning nil instead +// when the underlying driver rejects the input. +func FuzzHashToGWithDomain(f *testing.F) { + f.Add(0, []byte("hello world"), make([]byte, 256)) + f.Add(0, []byte(""), []byte("")) + f.Add(int(BLS12_381), []byte("data"), make([]byte, 255)) + f.Add(int(BLS12_381), []byte("data"), make([]byte, 256)) + f.Add(int(FP256BN_AMCL), []byte("data"), make([]byte, 300)) + + f.Fuzz(func(t *testing.T, curveIdx int, data, domain []byte) { + n := len(Curves) + idx := curveIdx % n + if idx < 0 { + idx += n + } + curve := Curves[idx] + + _ = curve.HashToG1WithDomain(data, domain) + _ = curve.HashToG2WithDomain(data, domain) + }) +} diff --git a/marshaler.go b/marshaler.go index edf9852..6afabef 100644 --- a/marshaler.go +++ b/marshaler.go @@ -6,13 +6,25 @@ SPDX-License-Identifier: Apache-2.0 package math -import "encoding/json" +import ( + "encoding/json" + "fmt" +) type curveElement struct { CurveID CurveID `json:"curve" validate:"required"` ElementBytes []byte `json:"element" validate:"required"` } +// validCurveID reports whether id can be safely used to index Curves. +func validCurveID(id CurveID) error { + if id < 0 || int(id) >= len(Curves) || Curves[id] == nil { + return fmt.Errorf("invalid curve id [%d]", id) + } + + return nil +} + func (z *Zr) UnmarshalJSON(raw []byte) error { ce := &curveElement{} err := json.Unmarshal(raw, ce) @@ -20,6 +32,10 @@ func (z *Zr) UnmarshalJSON(raw []byte) error { return err } + if err := validCurveID(ce.CurveID); err != nil { + return err + } + z.curveID = ce.CurveID z.zr = Curves[z.curveID].NewZrFromBytes(ce.ElementBytes).zr @@ -40,6 +56,10 @@ func (g *G1) UnmarshalJSON(raw []byte) error { return err } + if err := validCurveID(ce.CurveID); err != nil { + return err + } + g.curveID = ce.CurveID g1, err := Curves[g.curveID].NewG1FromBytes(ce.ElementBytes) if err != nil { @@ -65,6 +85,10 @@ func (g *G2) UnmarshalJSON(raw []byte) error { return err } + if err := validCurveID(ce.CurveID); err != nil { + return err + } + g.curveID = ce.CurveID g2, err := Curves[g.curveID].NewG2FromBytes(ce.ElementBytes) if err != nil { @@ -90,6 +114,10 @@ func (g *Gt) UnmarshalJSON(raw []byte) error { return err } + if err := validCurveID(ce.CurveID); err != nil { + return err + } + g.curveID = ce.CurveID gt, err := Curves[g.curveID].NewGtFromBytes(ce.ElementBytes) if err != nil { diff --git a/math.go b/math.go index 9440b22..ec47435 100644 --- a/math.go +++ b/math.go @@ -450,7 +450,10 @@ func (g *G1) Mul(a *Zr) *G1 { } // Mul2 computes [e]g + [f]Q and returns the result as a new G1 point. -// This is more efficient than computing the two scalar multiplications separately. +// On the gnark-backed curves this allocates far less than two separate Mul calls +// plus an Add, but it is not necessarily faster: the joint Strauss-Shamir technique +// used here forgoes the GLV endomorphism speedup that Mul benefits from, so wall-clock +// time is comparable to (not better than) the naive two-Mul-plus-Add approach. func (g *G1) Mul2(e *Zr, Q *G1, f *Zr) *G1 { return &G1{g1: g.g1.Mul2(e.zr, Q.g1, f.zr), curveID: g.curveID} } @@ -890,8 +893,18 @@ func (c *Curve) HashToG1(data []byte) *G1 { // HashToG1WithDomain hashes data to a G1 point with domain separation. // The domain parameter prevents hash collisions across different protocols or contexts. -func (c *Curve) HashToG1WithDomain(data, domain []byte) *G1 { - return &G1{g1: c.c.HashToG1WithDomain(data, domain), curveID: c.curveID} +// Returns nil if the underlying hash-to-curve algorithm rejects the input (e.g. a domain +// longer than 255 bytes), rather than panicking. +func (c *Curve) HashToG1WithDomain(data, domain []byte) (p *G1) { + defer func() { + if r := recover(); r != nil { + p = nil + } + }() + + p = &G1{g1: c.c.HashToG1WithDomain(data, domain), curveID: c.curveID} + + return } // HashToG2 hashes arbitrary data to a point in G2 using a hash-to-curve algorithm. @@ -901,8 +914,18 @@ func (c *Curve) HashToG2(data []byte) *G2 { // HashToG2WithDomain hashes data to a G2 point with domain separation. // The domain parameter prevents hash collisions across different protocols or contexts. -func (c *Curve) HashToG2WithDomain(data, domain []byte) *G2 { - return &G2{g2: c.c.HashToG2WithDomain(data, domain), curveID: c.curveID} +// Returns nil if the underlying hash-to-curve algorithm rejects the input (e.g. a domain +// longer than 255 bytes), rather than panicking. +func (c *Curve) HashToG2WithDomain(data, domain []byte) (p *G2) { + defer func() { + if r := recover(); r != nil { + p = nil + } + }() + + p = &G2{g2: c.c.HashToG2WithDomain(data, domain), curveID: c.curveID} + + return } // ModSub computes (a - b) mod m. diff --git a/math_test.go b/math_test.go index aa300c8..c43e0c0 100644 --- a/math_test.go +++ b/math_test.go @@ -849,6 +849,74 @@ func TestJSONMarshalerFails(t *testing.T) { require.EqualError(t, err, "failure [runtime error: index out of range [2] with length 2]") } +// TestJSONUnmarshalerInvalidCurveID is a regression test for a panic where an +// out-of-range or negative "curve" field in the JSON payload caused an unrecovered +// index-out-of-range panic (Curves[ce.CurveID]) instead of a returned error. +func TestJSONUnmarshalerInvalidCurveID(t *testing.T) { + invalidIDs := []int{-1, len(Curves), len(Curves) + 1000, 9999} + + for _, id := range invalidIDs { + payload := fmt.Appendf(nil, `{"curve": %d, "element": "AAAA"}`, id) + + zr := &Zr{} + err := zr.UnmarshalJSON(payload) + require.Error(t, err, "Zr.UnmarshalJSON should reject curve id %d", id) + + g1 := &G1{} + err = g1.UnmarshalJSON(payload) + require.Error(t, err, "G1.UnmarshalJSON should reject curve id %d", id) + + g2 := &G2{} + err = g2.UnmarshalJSON(payload) + require.Error(t, err, "G2.UnmarshalJSON should reject curve id %d", id) + + gt := &Gt{} + err = gt.UnmarshalJSON(payload) + require.Error(t, err, "Gt.UnmarshalJSON should reject curve id %d", id) + } + + // sanity check: every valid curve id must still be accepted. + for id := range Curves { + payload := fmt.Appendf(nil, `{"curve": %d, "element": "AAAA"}`, id) + zr := &Zr{} + err := zr.UnmarshalJSON(payload) + assert.NoError(t, err, "valid curve id %d should be accepted", id) + } +} + +// TestHashToGWithDomainOverlongDomain is a regression test for a panic in the +// gnark-backed drivers where a domain longer than 255 bytes caused +// HashToG1WithDomain/HashToG2WithDomain to panic instead of returning nil. +// AMCL-backed curves (FP256BN_AMCL, FP256BN_AMCL_MIRACL) are exercised too: +// their HashToG1WithDomain uses HMAC and never panics on domain length, and +// their HashToG2WithDomain panics unconditionally (regardless of domain +// length, see L1) which the curve-agnostic recover() in Curve.HashToG2WithDomain +// now converts into a nil return as well. +func TestHashToGWithDomainOverlongDomain(t *testing.T) { + data := []byte("hello world") + overlongDomain := make([]byte, 256) + + for _, curve := range Curves { + require.NotPanics(t, func() { + p := curve.HashToG1WithDomain(data, overlongDomain) + if curve.ID() != FP256BN_AMCL && curve.ID() != FP256BN_AMCL_MIRACL { + assert.Nil(t, p, "expected nil G1 for overlong domain on curve %v", curve.ID()) + } + }, "HashToG1WithDomain must not panic on curve %v", curve.ID()) + + require.NotPanics(t, func() { + curve.HashToG2WithDomain(data, overlongDomain) + }, "HashToG2WithDomain must not panic on curve %v", curve.ID()) + } + + // sanity check: a short, valid domain must still work on the curves that support G1 domain hashing. + shortDomain := []byte("my-domain") + for _, curve := range Curves { + p := curve.HashToG1WithDomain(data, shortDomain) + assert.NotNil(t, p, "expected non-nil G1 for short domain on curve %v", curve.ID()) + } +} + func TestCurves(t *testing.T) { for _, curve := range Curves { testNotZeroAfterAdd(t, curve)