Skip to content

Commit 4902cfb

Browse files
authored
fix: allow only v=0 or v=1 (#1293)
* fix: allow only v=0 or v=1 We followed the implementation of libsecp256k1 where v was allowed to be {0,1,2,3}. However, EVM specification has restricted allowed v to be only 0 or 1 for both transactions and precompiles. Resolves LA audit issue F. * test: add test case for unsatisfiable circuit if v∈{2,3}
1 parent ca8e156 commit 4902cfb

File tree

2 files changed

+42
-8
lines changed

2 files changed

+42
-8
lines changed

std/evmprecompiles/01-ecrecover.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66

77
"github.com/consensys/gnark/frontend"
88
"github.com/consensys/gnark/std/algebra/emulated/sw_emulated"
9-
"github.com/consensys/gnark/std/math/bits"
109
"github.com/consensys/gnark/std/math/emulated"
1110
)
1211

@@ -51,8 +50,8 @@ func ECRecover(api frontend.API, msg emulated.Element[emulated.Secp256k1Fr],
5150

5251
// EVM uses v \in {27, 28}, but everyone else v >= 0. Convert back
5352
v = api.Sub(v, 27)
54-
// check that len(v) = 2
55-
vbits := bits.ToBinary(api, v, bits.WithNbDigits(2))
53+
// check that len(v) = 1
54+
api.AssertIsBoolean(v)
5655

5756
// with the encoding we may have that r,s < 2*Fr (i.e. not r,s < Fr). Apply more thorough checks.
5857
frField.AssertIsLessOrEqual(&r, frField.Modulus())
@@ -90,10 +89,7 @@ func ECRecover(api frontend.API, msg emulated.Element[emulated.Secp256k1Fr],
9089
// compute R, the commitment
9190
// the signature as elements in Fr, but it actually represents elements in Fp. Convert to Fp element.
9291
rbits := frField.ToBits(&r)
93-
rfp := fpField.FromBits(rbits...)
94-
// compute R.X x = r+v[1]*fr
95-
Rx := fpField.Select(vbits[1], fpField.NewElement(emfr.Modulus()), fpField.NewElement(0))
96-
Rx = fpField.Add(rfp, Rx) // Rx = r + v[1]*fr
92+
Rx := fpField.FromBits(rbits...)
9793
Ry := fpField.Mul(Rx, Rx) // Ry = x^2
9894
// compute R.y y = sqrt(x^3+7)
9995
Ry = fpField.Mul(Ry, Rx) // Ry = x^3
@@ -104,7 +100,7 @@ func ECRecover(api frontend.API, msg emulated.Element[emulated.Secp256k1Fr],
104100
Ry = fpField.Sqrt(Ry) // Ry = sqrt(x^3 + 7)
105101
// ensure the oddity of Ry is same as vbits[0], otherwise negate Ry
106102
Rybits := fpField.ToBits(Ry)
107-
Ry = fpField.Select(api.Xor(vbits[0], Rybits[0]), fpField.Sub(fpField.Modulus(), Ry), Ry)
103+
Ry = fpField.Select(api.Xor(v, Rybits[0]), fpField.Sub(fpField.Modulus(), Ry), Ry)
108104

109105
R := sw_emulated.AffinePoint[emulated.Secp256k1Fp]{
110106
X: *Rx,

std/evmprecompiles/01-ecrecover_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package evmprecompiles
22

33
import (
44
"crypto/rand"
5+
"errors"
56
"fmt"
67
"math/big"
78
"testing"
@@ -260,3 +261,40 @@ func TestInvalidFailureTag(t *testing.T) {
260261
err := test.IsSolved(circuit, witness, ecc.BN254.ScalarField())
261262
assert.Error(err)
262263
}
264+
265+
func TestLargeV(t *testing.T) {
266+
assert := test.NewAssert(t)
267+
var pk ecdsa.PublicKey
268+
msg := []byte("test")
269+
var rE, sE fr.Element
270+
r, s := new(big.Int), new(big.Int)
271+
for _, v := range []uint{2, 3} {
272+
for {
273+
rE.SetRandom()
274+
sE.SetRandom()
275+
rE.BigInt(r)
276+
sE.BigInt(s)
277+
if err := pk.RecoverFrom(msg, v, r, s); errors.Is(err, ecdsa.ErrNoSqrtR) {
278+
continue
279+
} else {
280+
assert.NoError(err)
281+
break
282+
}
283+
}
284+
circuit := ecrecoverCircuit{}
285+
witness := ecrecoverCircuit{
286+
Message: emulated.ValueOf[emulated.Secp256k1Fr](ecdsa.HashToInt(msg)),
287+
V: v + 27, // EVM constant
288+
R: emulated.ValueOf[emulated.Secp256k1Fr](r),
289+
S: emulated.ValueOf[emulated.Secp256k1Fr](s),
290+
Strict: 0,
291+
IsFailure: 0,
292+
Expected: sw_emulated.AffinePoint[emulated.Secp256k1Fp]{
293+
X: emulated.ValueOf[emulated.Secp256k1Fp](pk.A.X),
294+
Y: emulated.ValueOf[emulated.Secp256k1Fp](pk.A.Y),
295+
},
296+
}
297+
err := test.IsSolved(&circuit, &witness, ecc.BLS12_377.ScalarField())
298+
assert.Error(err)
299+
}
300+
}

0 commit comments

Comments
 (0)