Skip to content

Commit

Permalink
fix: allow only v=0 or v=1 (#1293)
Browse files Browse the repository at this point in the history
* 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}
  • Loading branch information
ivokub authored Oct 10, 2024
1 parent ca8e156 commit 4902cfb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 8 deletions.
12 changes: 4 additions & 8 deletions std/evmprecompiles/01-ecrecover.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/consensys/gnark/frontend"
"github.com/consensys/gnark/std/algebra/emulated/sw_emulated"
"github.com/consensys/gnark/std/math/bits"
"github.com/consensys/gnark/std/math/emulated"
)

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

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

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

R := sw_emulated.AffinePoint[emulated.Secp256k1Fp]{
X: *Rx,
Expand Down
38 changes: 38 additions & 0 deletions std/evmprecompiles/01-ecrecover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package evmprecompiles

import (
"crypto/rand"
"errors"
"fmt"
"math/big"
"testing"
Expand Down Expand Up @@ -260,3 +261,40 @@ func TestInvalidFailureTag(t *testing.T) {
err := test.IsSolved(circuit, witness, ecc.BN254.ScalarField())
assert.Error(err)
}

func TestLargeV(t *testing.T) {
assert := test.NewAssert(t)
var pk ecdsa.PublicKey
msg := []byte("test")
var rE, sE fr.Element
r, s := new(big.Int), new(big.Int)
for _, v := range []uint{2, 3} {
for {
rE.SetRandom()
sE.SetRandom()
rE.BigInt(r)
sE.BigInt(s)
if err := pk.RecoverFrom(msg, v, r, s); errors.Is(err, ecdsa.ErrNoSqrtR) {
continue
} else {
assert.NoError(err)
break
}
}
circuit := ecrecoverCircuit{}
witness := ecrecoverCircuit{
Message: emulated.ValueOf[emulated.Secp256k1Fr](ecdsa.HashToInt(msg)),
V: v + 27, // EVM constant
R: emulated.ValueOf[emulated.Secp256k1Fr](r),
S: emulated.ValueOf[emulated.Secp256k1Fr](s),
Strict: 0,
IsFailure: 0,
Expected: sw_emulated.AffinePoint[emulated.Secp256k1Fp]{
X: emulated.ValueOf[emulated.Secp256k1Fp](pk.A.X),
Y: emulated.ValueOf[emulated.Secp256k1Fp](pk.A.Y),
},
}
err := test.IsSolved(&circuit, &witness, ecc.BLS12_377.ScalarField())
assert.Error(err)
}
}

0 comments on commit 4902cfb

Please sign in to comment.