crypto/secp256k1: fix undefined behavior in BitCurve.Add

crypto/secp256k1/curve.go

@@ -35,15 +35,8 @@ package secp256k1
 import (
 	"crypto/elliptic"
 	"math/big"
-	"unsafe"
 )
 
-/*
-#include "libsecp256k1/include/secp256k1.h"
-extern int secp256k1_ext_scalar_mul(const secp256k1_context* ctx, const unsigned char *point, const unsigned char *scalar);
-*/
-import "C"
-
 const (
 	// number of bits in a big.Word
 	wordBits = 32 << (uint64(^big.Word(0)) >> 63)
@@ -133,7 +126,18 @@ func (BitCurve *BitCurve) affineFromJacobian(x, y, z *big.Int) (xOut, yOut *big.
 
 // Add returns the sum of (x1,y1) and (x2,y2)
 func (BitCurve *BitCurve) Add(x1, y1, x2, y2 *big.Int) (*big.Int, *big.Int) {
+	// If one point is at infinity, return the other point.
+	// Adding the point at infinity to any point will preserve the other point.
+	if x1.Sign() == 0 && y1.Sign() == 0 {
+		return x2, y2
+	}
+	if x2.Sign() == 0 && y2.Sign() == 0 {
+		return x1, y1
+	}
 	z := new(big.Int).SetInt64(1)
+	if x1.Cmp(x2) == 0 && y1.Cmp(y2) == 0 {
+		return BitCurve.affineFromJacobian(BitCurve.doubleJacobian(x1, y1, z))
+	}
 	return BitCurve.affineFromJacobian(BitCurve.addJacobian(x1, y1, z, x2, y2, z))
 }
 
@@ -242,41 +246,6 @@ func (BitCurve *BitCurve) doubleJacobian(x, y, z *big.Int) (*big.Int, *big.Int,
 	return x3, y3, z3
 }
 
-func (BitCurve *BitCurve) ScalarMult(Bx, By *big.Int, scalar []byte) (*big.Int, *big.Int) {
-	// Ensure scalar is exactly 32 bytes. We pad always, even if
-	// scalar is 32 bytes long, to avoid a timing side channel.
-	if len(scalar) > 32 {
-		panic("can't handle scalars > 256 bits")
-	}
-	// NOTE: potential timing issue
-	padded := make([]byte, 32)
-	copy(padded[32-len(scalar):], scalar)
-	scalar = padded
-
-	// Do the multiplication in C, updating point.
-	point := make([]byte, 64)
-	readBits(Bx, point[:32])
-	readBits(By, point[32:])
-
-	pointPtr := (*C.uchar)(unsafe.Pointer(&point[0]))
-	scalarPtr := (*C.uchar)(unsafe.Pointer(&scalar[0]))
-	res := C.secp256k1_ext_scalar_mul(context, pointPtr, scalarPtr)
-
-	// Unpack the result and clear temporaries.
-	x := new(big.Int).SetBytes(point[:32])
-	y := new(big.Int).SetBytes(point[32:])
-	for i := range point {
-		point[i] = 0
-	}
-	for i := range padded {
-		scalar[i] = 0
-	}
-	if res != 1 {
-		return nil, nil
-	}
-	return x, y
-}
-
 // ScalarBaseMult returns k*G, where G is the base point of the group and k is
 // an integer in big-endian form.
 func (BitCurve *BitCurve) ScalarBaseMult(k []byte) (*big.Int, *big.Int) {

crypto/secp256k1/panic_cb.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be found in
 // the LICENSE file.
 
+// +build !gofuzz cgo
+
 package secp256k1
 
 import "C"

crypto/secp256k1/scalar_mult_cgo.go

@@ -0,0 +1,56 @@
+// Copyright 2015 Jeffrey Wilcke, Felix Lange, Gustav Simonsson. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+
+// +build !gofuzz cgo
+
+package secp256k1
+
+import (
+	"math/big"
+	"unsafe"
+)
+
+/*
+
+#include "libsecp256k1/include/secp256k1.h"
+
+extern int secp256k1_ext_scalar_mul(const secp256k1_context* ctx, const unsigned char *point, const unsigned char *scalar);
+
+*/
+import "C"
+
+func (BitCurve *BitCurve) ScalarMult(Bx, By *big.Int, scalar []byte) (*big.Int, *big.Int) {
+	// Ensure scalar is exactly 32 bytes. We pad always, even if
+	// scalar is 32 bytes long, to avoid a timing side channel.
+	if len(scalar) > 32 {
+		panic("can't handle scalars > 256 bits")
+	}
+	// NOTE: potential timing issue
+	padded := make([]byte, 32)
+	copy(padded[32-len(scalar):], scalar)
+	scalar = padded
+
+	// Do the multiplication in C, updating point.
+	point := make([]byte, 64)
+	readBits(Bx, point[:32])
+	readBits(By, point[32:])
+
+	pointPtr := (*C.uchar)(unsafe.Pointer(&point[0]))
+	scalarPtr := (*C.uchar)(unsafe.Pointer(&scalar[0]))
+	res := C.secp256k1_ext_scalar_mul(context, pointPtr, scalarPtr)
+
+	// Unpack the result and clear temporaries.
+	x := new(big.Int).SetBytes(point[:32])
+	y := new(big.Int).SetBytes(point[32:])
+	for i := range point {
+		point[i] = 0
+	}
+	for i := range padded {
+		scalar[i] = 0
+	}
+	if res != 1 {
+		return nil, nil
+	}
+	return x, y
+}

crypto/secp256k1/scalar_mult_nocgo.go

@@ -0,0 +1,13 @@
+// Copyright 2015 Jeffrey Wilcke, Felix Lange, Gustav Simonsson. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+
+// +build gofuzz !cgo
+
+package secp256k1
+
+import "math/big"
+
+func (BitCurve *BitCurve) ScalarMult(Bx, By *big.Int, scalar []byte) (*big.Int, *big.Int) {
+	panic("ScalarMult is not available when secp256k1 is built without cgo")
+}

crypto/secp256k1/secp256.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be found in
 // the LICENSE file.
 
+// +build !gofuzz cgo
+
 // Package secp256k1 wraps the bitcoin secp256k1 C library.
 package secp256k1
 

oss-fuzz.sh

@@ -102,6 +102,7 @@ compile_fuzzer tests/fuzzers/stacktrie  Fuzz fuzzStackTrie
 compile_fuzzer tests/fuzzers/difficulty Fuzz fuzzDifficulty
 compile_fuzzer tests/fuzzers/abi        Fuzz fuzzAbi
 compile_fuzzer tests/fuzzers/les        Fuzz fuzzLes
+compile_fuzzer tests/fuzzers/secp265k1  Fuzz fuzzSecp256k1
 compile_fuzzer tests/fuzzers/vflux      FuzzClientPool fuzzClientPool
 
 compile_fuzzer tests/fuzzers/bls12381  FuzzG1Add fuzz_g1_add

tests/fuzzers/secp256k1/secp_fuzzer.go

@@ -0,0 +1,50 @@
+// Copyright 2021 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+// build +gofuzz
+
+package secp256k1
+
+import (
+	"fmt"
+
+	"github.com/btcsuite/btcd/btcec"
+	"github.com/ethereum/go-ethereum/crypto/secp256k1"
+	fuzz "github.com/google/gofuzz"
+)
+
+func Fuzz(input []byte) int {
+	var (
+		fuzzer = fuzz.NewFromGoFuzz(input)
+		curveA = secp256k1.S256()
+		curveB = btcec.S256()
+		dataP1 []byte
+		dataP2 []byte
+	)
+	// first point
+	fuzzer.Fuzz(&dataP1)
+	x1, y1 := curveB.ScalarBaseMult(dataP1)
+	// second point
+	fuzzer.Fuzz(&dataP2)
+	x2, y2 := curveB.ScalarBaseMult(dataP2)
+	resAX, resAY := curveA.Add(x1, y1, x2, y2)
+	resBX, resBY := curveB.Add(x1, y1, x2, y2)
+	if resAX.Cmp(resBX) != 0 || resAY.Cmp(resBY) != 0 {
+		fmt.Printf("%s %s %s %s\n", x1, y1, x2, y2)
+		panic(fmt.Sprintf("Addition failed: geth: %s %s btcd: %s %s", resAX, resAY, resBX, resBY))
+	}
+	return 0
+}

tests/fuzzers/secp256k1/secp_test.go

@@ -0,0 +1,8 @@
+package secp256k1
+
+import "testing"
+
+func TestFuzzer(t *testing.T) {
+	test := "00000000N0000000/R00000000000000000U0000S0000000mkhP000000000000000U"
+	Fuzz([]byte(test))
+}