Merge pull request #398 from XinFinOrg/XDC-01

XDC-01 | Potential Missed Fixings in `crypto` Module

crypto/bn256/bn256_fuzz.go

@@ -20,42 +20,52 @@ package bn256
 
 import (
 	"bytes"
+	"fmt"
+	"io"
 	"math/big"
 
 	cloudflare "github.com/XinFinOrg/XDPoSChain/crypto/bn256/cloudflare"
 	google "github.com/XinFinOrg/XDPoSChain/crypto/bn256/google"
 )
 
+func getG1Points(input io.Reader) (*cloudflare.G1, *google.G1) {
+	_, xc, err := cloudflare.RandomG1(input)
+	if err != nil {
+		// insufficient input
+		return nil, nil
+	}
+	xg := new(google.G1)
+	if _, err := xg.Unmarshal(xc.Marshal()); err != nil {
+		panic(fmt.Sprintf("Could not marshal cloudflare -> google:", err))
+	}
+	return xc, xg
+}
+
+func getG2Points(input io.Reader) (*cloudflare.G2, *google.G2) {
+	_, xc, err := cloudflare.RandomG2(input)
+	if err != nil {
+		// insufficient input
+		return nil, nil
+	}
+	xg := new(google.G2)
+	if _, err := xg.Unmarshal(xc.Marshal()); err != nil {
+		panic(fmt.Sprintf("Could not marshal cloudflare -> google:", err))
+	}
+	return xc, xg
+}
+
 // FuzzAdd fuzzez bn256 addition between the Google and Cloudflare libraries.
 func FuzzAdd(data []byte) int {
-	// Ensure we have enough data in the first place
+	input := bytes.NewReader(data)
-	if len(data) != 128 {
+	xc, xg := getG1Points(input)
+	if xc == nil {
 		return 0
 	}
-	// Ensure both libs can parse the first curve point
+	yc, yg := getG1Points(input)
-	xc := new(cloudflare.G1)
+	if yc == nil {
-	_, errc := xc.Unmarshal(data[:64])
-
-	xg := new(google.G1)
-	_, errg := xg.Unmarshal(data[:64])
-
-	if (errc == nil) != (errg == nil) {
-		panic("parse mismatch")
-	} else if errc != nil {
 		return 0
 	}
 	// Ensure both libs can parse the second curve point
-	yc := new(cloudflare.G1)
-	_, errc = yc.Unmarshal(data[64:])
-
-	yg := new(google.G1)
-	_, errg = yg.Unmarshal(data[64:])
-
-	if (errc == nil) != (errg == nil) {
-		panic("parse mismatch")
-	} else if errc != nil {
-		return 0
-	}
 	// Add the two points and ensure they result in the same output
 	rc := new(cloudflare.G1)
 	rc.Add(xc, yc)
@@ -66,73 +76,50 @@ func FuzzAdd(data []byte) int {
 	if !bytes.Equal(rc.Marshal(), rg.Marshal()) {
 		panic("add mismatch")
 	}
-	return 0
+	return 1
 }
 
 // FuzzMul fuzzez bn256 scalar multiplication between the Google and Cloudflare
 // libraries.
 func FuzzMul(data []byte) int {
-	// Ensure we have enough data in the first place
+	input := bytes.NewReader(data)
-	if len(data) != 96 {
+	pc, pg := getG1Points(input)
-		return 0
+	if pc == nil {
-	}
-	// Ensure both libs can parse the curve point
-	pc := new(cloudflare.G1)
-	_, errc := pc.Unmarshal(data[:64])
-
-	pg := new(google.G1)
-	_, errg := pg.Unmarshal(data[:64])
-
-	if (errc == nil) != (errg == nil) {
-		panic("parse mismatch")
-	} else if errc != nil {
 		return 0
 	}
 	// Add the two points and ensure they result in the same output
+	remaining := input.Len()
+	if remaining == 0 {
+		return 0
+	}
+	buf := make([]byte, remaining)
+	input.Read(buf)
+
 	rc := new(cloudflare.G1)
-	rc.ScalarMult(pc, new(big.Int).SetBytes(data[64:]))
+	rc.ScalarMult(pc, new(big.Int).SetBytes(buf))
 
 	rg := new(google.G1)
-	rg.ScalarMult(pg, new(big.Int).SetBytes(data[64:]))
+	rg.ScalarMult(pg, new(big.Int).SetBytes(buf))
 
 	if !bytes.Equal(rc.Marshal(), rg.Marshal()) {
 		panic("scalar mul mismatch")
 	}
-	return 0
+	return 1
 }
 
 func FuzzPair(data []byte) int {
-	// Ensure we have enough data in the first place
+	input := bytes.NewReader(data)
-	if len(data) != 192 {
+	pc, pg := getG1Points(input)
+	if pc == nil {
 		return 0
 	}
-	// Ensure both libs can parse the curve point
+	tc, tg := getG2Points(input)
-	pc := new(cloudflare.G1)
+	if tc == nil {
-	_, errc := pc.Unmarshal(data[:64])
-
-	pg := new(google.G1)
-	_, errg := pg.Unmarshal(data[:64])
-
-	if (errc == nil) != (errg == nil) {
-		panic("parse mismatch")
-	} else if errc != nil {
-		return 0
-	}
-	// Ensure both libs can parse the twist point
-	tc := new(cloudflare.G2)
-	_, errc = tc.Unmarshal(data[64:])
-
-	tg := new(google.G2)
-	_, errg = tg.Unmarshal(data[64:])
-
-	if (errc == nil) != (errg == nil) {
-		panic("parse mismatch")
-	} else if errc != nil {
 		return 0
 	}
 	// Pair the two points and ensure thet result in the same output
 	if cloudflare.PairingCheck([]*cloudflare.G1{pc}, []*cloudflare.G2{tc}) != google.PairingCheck([]*google.G1{pg}, []*google.G2{tg}) {
 		panic("pair mismatch")
 	}
-	return 0
+	return 1
 }

crypto/bn256/cloudflare/bn256.go

@@ -23,7 +23,7 @@ import (
 func randomK(r io.Reader) (k *big.Int, err error) {
 	for {
 		k, err = rand.Int(r, Order)
-		if k.Sign() > 0 || err != nil {
+		if err != nil || k.Sign() > 0 {
 			return
 		}
 	}
@@ -100,6 +100,10 @@ func (e *G1) Marshal() []byte {
 	// Each value is a 256-bit number.
 	const numBytes = 256 / 8
 
+	if e.p == nil {
+		e.p = &curvePoint{}
+	}
+
 	e.p.MakeAffine()
 	ret := make([]byte, numBytes*2)
 	if e.p.IsInfinity() {
@@ -382,6 +386,11 @@ func (e *GT) Marshal() []byte {
 	// Each value is a 256-bit number.
 	const numBytes = 256 / 8
 
+	if e.p == nil {
+		e.p = &gfP12{}
+		e.p.SetOne()
+	}
+
 	ret := make([]byte, numBytes*12)
 	temp := &gfP{}
 

crypto/bn256/cloudflare/bn256_test.go

@@ -92,6 +92,19 @@ func TestTripartiteDiffieHellman(t *testing.T) {
 	}
 }
 
+func TestG2SelfAddition(t *testing.T) {
+	s, _ := rand.Int(rand.Reader, Order)
+	p := new(G2).ScalarBaseMult(s)
+
+	if !p.p.IsOnCurve() {
+		t.Fatal("p isn't on curve")
+	}
+	m := p.Add(p, p).Marshal()
+	if _, err := p.Unmarshal(m); err != nil {
+		t.Fatalf("p.Add(p, p) ∉ G₂: %v", err)
+	}
+}
+
 func BenchmarkG1(b *testing.B) {
 	x, _ := rand.Int(rand.Reader, Order)
 	b.ResetTimer()

crypto/bn256/cloudflare/curve.go

@@ -171,15 +171,15 @@ func (c *curvePoint) Double(a *curvePoint) {
 	gfpAdd(t, d, d)
 	gfpSub(&c.x, f, t)
 
+	gfpMul(&c.z, &a.y, &a.z)
+	gfpAdd(&c.z, &c.z, &c.z)
+
 	gfpAdd(t, C, C)
 	gfpAdd(t2, t, t)
 	gfpAdd(t, t2, t2)
 	gfpSub(&c.y, d, &c.x)
 	gfpMul(t2, e, &c.y)
 	gfpSub(&c.y, t2, t)
-
-	gfpMul(t, &a.y, &a.z)
-	gfpAdd(&c.z, t, t)
 }
 
 func (c *curvePoint) Mul(a *curvePoint, scalar *big.Int) {

crypto/bn256/cloudflare/gfp.go

@@ -61,6 +61,7 @@ func (e *gfP) Marshal(out []byte) {
 func (e *gfP) Unmarshal(in []byte) error {
 	// Unmarshal the bytes into little endian form
 	for w := uint(0); w < 4; w++ {
+		e[3-w] = 0
 		for b := uint(0); b < 8; b++ {
 			e[3-w] += uint64(in[8*w+b]) << (56 - 8*b)
 		}

crypto/bn256/cloudflare/gfp_amd64.s

@@ -49,7 +49,7 @@ TEXT ·gfpNeg(SB),0,$0-16
 	SBBQ 24(DI), R11
 
 	MOVQ $0, AX
-	gfpCarry(R8,R9,R10,R11,AX, R12,R13,R14,R15,BX)
+	gfpCarry(R8,R9,R10,R11,AX, R12,R13,R14,CX,BX)
 
 	MOVQ c+0(FP), DI
 	storeBlock(R8,R9,R10,R11, 0(DI))
@@ -68,7 +68,7 @@ TEXT ·gfpAdd(SB),0,$0-24
 	ADCQ 24(SI), R11
 	ADCQ $0, R12
 
-	gfpCarry(R8,R9,R10,R11,R12, R13,R14,R15,AX,BX)
+	gfpCarry(R8,R9,R10,R11,R12, R13,R14,CX,AX,BX)
 
 	MOVQ c+0(FP), DI
 	storeBlock(R8,R9,R10,R11, 0(DI))
@@ -83,7 +83,7 @@ TEXT ·gfpSub(SB),0,$0-24
 	MOVQ ·p2+0(SB), R12
 	MOVQ ·p2+8(SB), R13
 	MOVQ ·p2+16(SB), R14
-	MOVQ ·p2+24(SB), R15
+	MOVQ ·p2+24(SB), CX
 	MOVQ $0, AX
 
 	SUBQ  0(SI), R8
@@ -94,12 +94,12 @@ TEXT ·gfpSub(SB),0,$0-24
 	CMOVQCC AX, R12
 	CMOVQCC AX, R13
 	CMOVQCC AX, R14
-	CMOVQCC AX, R15
+	CMOVQCC AX, CX
 
 	ADDQ R12, R8
 	ADCQ R13, R9
 	ADCQ R14, R10
-	ADCQ R15, R11
+	ADCQ CX, R11
 
 	MOVQ c+0(FP), DI
 	storeBlock(R8,R9,R10,R11, 0(DI))
@@ -115,7 +115,7 @@ TEXT ·gfpMul(SB),0,$160-24
 
 	mulBMI2(0(DI),8(DI),16(DI),24(DI), 0(SI))
 	storeBlock( R8, R9,R10,R11,  0(SP))
-	storeBlock(R12,R13,R14,R15, 32(SP))
+	storeBlock(R12,R13,R14,CX, 32(SP))
 	gfpReduceBMI2()
 	JMP end
 
@@ -125,6 +125,6 @@ nobmi2Mul:
 
 end:
 	MOVQ c+0(FP), DI
-	storeBlock(R12,R13,R14,R15, 0(DI))
+	storeBlock(R12,R13,R14,CX, 0(DI))
 	RET
 

crypto/bn256/cloudflare/mul_amd64.h

@@ -165,7 +165,7 @@
 	\
 	\ // Add the 512-bit intermediate to m*N
 	loadBlock(96+stack, R8,R9,R10,R11) \
-	loadBlock(128+stack, R12,R13,R14,R15) \
+	loadBlock(128+stack, R12,R13,R14,CX) \
 	\
 	MOVQ $0, AX \
 	ADDQ 0+stack, R8 \
@@ -175,7 +175,7 @@
 	ADCQ 32+stack, R12 \
 	ADCQ 40+stack, R13 \
 	ADCQ 48+stack, R14 \
-	ADCQ 56+stack, R15 \
+	ADCQ 56+stack, CX \
 	ADCQ $0, AX \
 	\
-	gfpCarry(R12,R13,R14,R15,AX, R8,R9,R10,R11,BX)
+	gfpCarry(R12,R13,R14,CX,AX, R8,R9,R10,R11,BX)

crypto/bn256/cloudflare/mul_bmi2_amd64.h

@@ -29,7 +29,7 @@
 	ADCQ $0, R14 \
 	\
 	MOVQ a2, DX \
-	MOVQ $0, R15 \
+	MOVQ $0, CX \
 	MULXQ 0+rb, AX, BX \
 	ADDQ AX, R10 \
 	ADCQ BX, R11 \
@@ -43,7 +43,7 @@
 	MULXQ 24+rb, AX, BX \
 	ADCQ AX, R13 \
 	ADCQ BX, R14 \
-	ADCQ $0, R15 \
+	ADCQ $0, CX \
 	\
 	MOVQ a3, DX \
 	MULXQ 0+rb, AX, BX \
@@ -52,13 +52,13 @@
 	MULXQ 16+rb, AX, BX \
 	ADCQ AX, R13 \
 	ADCQ BX, R14 \
-	ADCQ $0, R15 \
+	ADCQ $0, CX \
 	MULXQ 8+rb, AX, BX \
 	ADDQ AX, R12 \
 	ADCQ BX, R13 \
 	MULXQ 24+rb, AX, BX \
 	ADCQ AX, R14 \
-	ADCQ BX, R15
+	ADCQ BX, CX
 
 #define gfpReduceBMI2() \
 	\ // m = (T * N') mod R, store m in R8:R9:R10:R11
@@ -106,7 +106,7 @@
 	ADCQ 32(SP), R12 \
 	ADCQ 40(SP), R13 \
 	ADCQ 48(SP), R14 \
-	ADCQ 56(SP), R15 \
+	ADCQ 56(SP), CX \
 	ADCQ $0, AX \
 	\
-	gfpCarry(R12,R13,R14,R15,AX, R8,R9,R10,R11,BX)
+	gfpCarry(R12,R13,R14,CX,AX, R8,R9,R10,R11,BX)

crypto/bn256/cloudflare/twist.go

@@ -150,15 +150,15 @@ func (c *twistPoint) Double(a *twistPoint) {
 	t.Add(d, d)
 	c.x.Sub(f, t)
 
+	c.z.Mul(&a.y, &a.z)
+	c.z.Add(&c.z, &c.z)
+
 	t.Add(C, C)
 	t2.Add(t, t)
 	t.Add(t2, t2)
 	c.y.Sub(d, &c.x)
 	t2.Mul(e, &c.y)
 	c.y.Sub(t2, t)
-
-	t.Mul(&a.y, &a.z)
-	c.z.Add(t, t)
 }
 
 func (c *twistPoint) Mul(a *twistPoint, scalar *big.Int) {

crypto/secp256k1/curve.go

@@ -96,6 +96,10 @@ func (BitCurve *BitCurve) IsOnCurve(x, y *big.Int) bool {
 // affineFromJacobian reverses the Jacobian transform. See the comment at the
 // top of the file.
 func (BitCurve *BitCurve) affineFromJacobian(x, y, z *big.Int) (xOut, yOut *big.Int) {
+	if z.Sign() == 0 {
+		return new(big.Int), new(big.Int)
+	}
+
 	zinv := new(big.Int).ModInverse(z, BitCurve.P)
 	zinvsq := new(big.Int).Mul(zinv, zinv)