From 3d635c544e7e96a24c972e4f5f5710ead8ffca84 Mon Sep 17 00:00:00 2001 From: Abd ar-Rahman Hamidi Date: Tue, 17 Nov 2020 15:47:17 +0500 Subject: [PATCH 1/5] crypto/secp256k1: add checking z sign in affineFromJacobian (#18419) The z == 0 check is hit whenever we Add two points with the same x1/x2 coordinate. crypto/elliptic uses the same check in their affineFromJacobian function. This change does not affect block processing or tx signature verification in any way, because it does not use the Add or Double methods. --- crypto/secp256k1/curve.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crypto/secp256k1/curve.go b/crypto/secp256k1/curve.go index a12752e77b..ecf7a238f0 100644 --- a/crypto/secp256k1/curve.go +++ b/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) From c48e9873f3e27ca050b4d4c770a255e4645eb72f Mon Sep 17 00:00:00 2001 From: Antoine Rondelet Date: Sat, 25 May 2019 22:57:07 +0100 Subject: [PATCH 2/5] crypto/bn256/cloudflare: checks for nil pointers in Marshal functions (#19609) * Added checks for nil pointers in Marshal functions * Set nil pointer to identity in GT before marshaling --- crypto/bn256/cloudflare/bn256.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crypto/bn256/cloudflare/bn256.go b/crypto/bn256/cloudflare/bn256.go index c6ea2d07e0..38822a76bf 100644 --- a/crypto/bn256/cloudflare/bn256.go +++ b/crypto/bn256/cloudflare/bn256.go @@ -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{} From 3a6e26b844a8a552efbc0763796a72f431010826 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 13 Nov 2020 09:27:57 +0100 Subject: [PATCH 3/5] crypto/bn256: improve bn256 fuzzer (#21815) * crypto/cloudflare: fix nil deref in random G1/G2 reading * crypto/bn256: improve fuzzer * crypto/bn256: fix some flaws in fuzzer --- crypto/bn256/bn256_fuzz.go | 119 ++++++++++++++----------------- crypto/bn256/cloudflare/bn256.go | 2 +- 2 files changed, 54 insertions(+), 67 deletions(-) diff --git a/crypto/bn256/bn256_fuzz.go b/crypto/bn256/bn256_fuzz.go index eec0858466..c3e1833ef2 100644 --- a/crypto/bn256/bn256_fuzz.go +++ b/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 - if len(data) != 128 { + input := bytes.NewReader(data) + xc, xg := getG1Points(input) + if xc == nil { return 0 } - // Ensure both libs can parse the first curve point - xc := new(cloudflare.G1) - _, 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 { + yc, yg := getG1Points(input) + if yc == 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 - if len(data) != 96 { - return 0 - } - // 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 { + input := bytes.NewReader(data) + pc, pg := getG1Points(input) + if pc == 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 - if len(data) != 192 { + input := bytes.NewReader(data) + pc, pg := getG1Points(input) + if pc == nil { return 0 } - // 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 - } - // 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 { + tc, tg := getG2Points(input) + if tc == 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 } diff --git a/crypto/bn256/cloudflare/bn256.go b/crypto/bn256/cloudflare/bn256.go index 38822a76bf..a6dd972ba8 100644 --- a/crypto/bn256/cloudflare/bn256.go +++ b/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 } } From af8c4d4e85a9fb74ccdebd5b9057545337093163 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 25 Aug 2021 17:33:09 +0200 Subject: [PATCH 4/5] crypto/cloudflare/bn256: fix in-place addition and unmarshalling (#23419) --- crypto/bn256/cloudflare/bn256_test.go | 13 +++++++++++++ crypto/bn256/cloudflare/curve.go | 6 +++--- crypto/bn256/cloudflare/gfp.go | 1 + crypto/bn256/cloudflare/twist.go | 6 +++--- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/crypto/bn256/cloudflare/bn256_test.go b/crypto/bn256/cloudflare/bn256_test.go index 0c8016d86c..481e2f78c3 100644 --- a/crypto/bn256/cloudflare/bn256_test.go +++ b/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() diff --git a/crypto/bn256/cloudflare/curve.go b/crypto/bn256/cloudflare/curve.go index 18e9b38f3f..16f0489e33 100644 --- a/crypto/bn256/cloudflare/curve.go +++ b/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) { diff --git a/crypto/bn256/cloudflare/gfp.go b/crypto/bn256/cloudflare/gfp.go index e8e84e7b3b..b15e1697e1 100644 --- a/crypto/bn256/cloudflare/gfp.go +++ b/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) } diff --git a/crypto/bn256/cloudflare/twist.go b/crypto/bn256/cloudflare/twist.go index 0c2f80d4ed..2c7a69a4d7 100644 --- a/crypto/bn256/cloudflare/twist.go +++ b/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) { From d4b98063e8bd536539e38de4db56c165af4420e1 Mon Sep 17 00:00:00 2001 From: uji <49834542+uji@users.noreply.github.com> Date: Wed, 9 Mar 2022 08:23:13 +0900 Subject: [PATCH 5/5] crypto/bn256/cloudflare: fix asm for dynamic linking (#24476) When using -buildmode=shared, R15 is clobbered by a global variable access; use a different register instead. Fixes: #24439 --- crypto/bn256/cloudflare/gfp_amd64.s | 14 +++++++------- crypto/bn256/cloudflare/mul_amd64.h | 6 +++--- crypto/bn256/cloudflare/mul_bmi2_amd64.h | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crypto/bn256/cloudflare/gfp_amd64.s b/crypto/bn256/cloudflare/gfp_amd64.s index e7d430971c..8c5fe23fb0 100644 --- a/crypto/bn256/cloudflare/gfp_amd64.s +++ 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 diff --git a/crypto/bn256/cloudflare/mul_amd64.h b/crypto/bn256/cloudflare/mul_amd64.h index bab5da8313..9d8e4b37db 100644 --- a/crypto/bn256/cloudflare/mul_amd64.h +++ b/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) diff --git a/crypto/bn256/cloudflare/mul_bmi2_amd64.h b/crypto/bn256/cloudflare/mul_bmi2_amd64.h index 71ad0499af..403566c6fa 100644 --- a/crypto/bn256/cloudflare/mul_bmi2_amd64.h +++ b/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)