Skip to content

Commit

Permalink
Fix unmarshal bugs (#409)
Browse files Browse the repository at this point in the history 
* [evm precompiles] do *not* ignore `unmarshal` return value

Because `unmarshal` is marked as `discardable` we silently ignored the
return value of `unmarshal`, which in some test cases indicated that
the source value does not fit into target bigint

* [io_limbs] handle `unmarshal` for sources longer, but smaller

If a given source `src` is given to `unmarshal` where the most
significant bytes are all zero beyond the size that fits into the
destination BigInt, we still return a successful parse. Previously we
would just return false whenever we found more bytes than can possibly
fit (this leads to issues with e.g. EVM precompiles tests, which come
as strings that are longer).

Note: We can probably do the all zero check in a better
way (i.e. using the fact that endianness does not matter and just
compare all relevant bytes, but not sure about the most elegant way to
do that, taking the dynamic sizes into account)

* [tests] explicitly pass output size for EVM precompiles, chk exp err

* revert checking of "upper" bytes in `io_limbs.nim`

* [evm precompiles] return invalid input size for MSM input.len == 0

* [evm precompiles] split parsing of field coords by curve

For BLS12-381 we now parse according to the spec
https://eips.ethereum.org/EIPS/eip-2537#fine-points-and-encoding-of-base-elements
making sure to check the 'upper' 16 bytes to be empty. If they are not
we return IntLargerThanModulus.
  • Loading branch information
@Vindaar
Vindaar authored Jul 5, 2024
1 parent 463c696 commit ad325cf
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 39 deletions.
56 changes: 48 additions & 8 deletions constantine/ethereum_evm_precompiles.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,19 +196,59 @@ func eth_evm_modexp*(r: var openArray[byte], inputs: openArray[byte]): CttEVMSta
# Elliptic Curves
# ----------------------------------------------------------------

func parseRawUint[Name: static Algebra](
dst: var Fp[Name],
src: openarray[byte]): CttEVMStatus =
proc parseEip2537(dst: var Fp[BLS12_381], src: openArray[byte]): CttEvmStatus {.inline.} =
## Parses a curve point following the encoding rules defined in EIP-2537,
## i.e. requiring the input to be exactly 64 bytes, with the 'upper' 16 bytes
## required to be empty.
## The input is required to be a coordinate of a point on the BLS12-381 curve and
## stored in Big Endian format.
##
## For points on the quadratic extension field Fp2 the individual terms are
## byte concatenated and the caller takes care of splitting the input for a
## before calling this.
##
## Ref: https://eips.ethereum.org/EIPS/eip-2537#fine-points-and-encoding-of-base-elements

var big {.noInit.}: Fp[BLS12_381].getBigInt()
if src.len != 64:
return cttEVM_InvalidInputSize

# Parse the subslice of 48 bytes using regular `unmarshal`
var success = big.unmarshal(src.toOpenArray(16, 63), bigEndian)

# Now check that all lower bytes are empty
var allZero = success # if `success` already false we still continue
for i in 0 ..< 15: # order irrelevant
allZero = allZero and (src[i] == 0)

if not allZero or not bool(big < Fp[BLS12_381].getModulus()):
return cttEVM_IntLargerThanModulus

# assign result
dst.fromBig(big)
result = cttEVM_Success

func parseRawUint(dst: var Fp[BLS12_381], src: openarray[byte]): CttEVMStatus =
## Parse an unsigned integer from its canonical
## big-endian or little-endian unsigned representation
## And store it into a field element for
##
## Return false if the integer is larger than the field modulus.
## Returns true on success.
result = dst.parseEip2537(src)

func parseRawUint(dst: var Fp[BN254_Snarks], src: openarray[byte]): CttEVMStatus =
## Parse an unsigned integer from its canonical
## big-endian or little-endian unsigned representation
## And store it into a field element.
##
## Return false if the integer is larger than the field modulus.
## Returns true on success.
var big {.noInit.}: Fp[Name].getBigInt()
big.unmarshal(src, bigEndian)
var big {.noInit.}: Fp[BN254_Snarks].getBigInt()
if not big.unmarshal(src, bigEndian):
return cttEVM_IntLargerThanModulus # `dst` too small for `src`!

if not bool(big < Fp[Name].getModulus()):
if not bool(big < Fp[BN254_Snarks].getModulus()):
return cttEVM_IntLargerThanModulus

dst.fromBig(big)
Expand Down Expand Up @@ -819,7 +859,7 @@ func eth_evm_bls12381_g1msm*(r: var openArray[byte], inputs: openarray[byte]): C
## cttEVM_PointNotOnCurve
##
## Spec https://eips.ethereum.org/EIPS/eip-2537
if inputs.len mod 160 != 0:
if inputs.len == 0 or inputs.len mod 160 != 0:
return cttEVM_InvalidInputSize

if r.len != 128:
Expand Down Expand Up @@ -902,7 +942,7 @@ func eth_evm_bls12381_g2msm*(r: var openArray[byte], inputs: openarray[byte]): C
## cttEVM_PointNotOnCurve
##
## Spec https://eips.ethereum.org/EIPS/eip-2537
if inputs.len mod 288 != 0:
if inputs.len == 0 or inputs.len mod 288 != 0:
return cttEVM_InvalidInputSize

if r.len != 256:
Expand Down
70 changes: 39 additions & 31 deletions tests/t_ethereum_evm_precompiles.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type
PrecompileTest = object
Input: HexString
Expected: HexString
ExpectedError: string
Name: string
Gas: int
NoBenchmark: bool
Expand All @@ -37,7 +38,7 @@ proc loadVectors(TestType: typedesc, filename: string): TestType =
let content = readFile(TestVectorsDir/filename)
result = content.fromJson(TestType)

template runPrecompileTests(filename: string, funcname: untyped) =
template runPrecompileTests(filename: string, funcname: untyped, outsize: int) =
block:
proc `PrecompileTestrunner _ funcname`() =
let vec = seq[PrecompileTest].loadVectors(filename)
Expand All @@ -53,16 +54,23 @@ template runPrecompileTests(filename: string, funcname: untyped) =
var expected = newSeq[byte](test.Expected.len div 2)
expected.paddedFromHex(test.Expected, bigEndian)

var r = newSeq[byte](test.Expected.len div 2)
## TODO: change to use `modexp_result_size` API after rebase
let outs = if outsize > 0: outsize else: test.Expected.len div 2
var r = newSeq[byte](outs)

let status = funcname(r, inputbytes)
if status != cttEVM_Success:
reset(r)

doAssert r == expected, "[Test Failure]\n" &
" " & funcname.astToStr & " status: " & $status & "\n" &
" " & "result: " & r.toHex() & "\n" &
" " & "expected: " & expected.toHex() & '\n'
doAssert test.ExpectedError.len > 0, "[Test Failure]\n" &
" " & test.Name & "\n" &
" " & funcname.astToStr & "\n" &
" " & "Nim proc returned failure, but test expected to pass.\n" &
" " & "Expected result: " & $expected.toHex()
else:
doAssert r == expected, "[Test Failure]\n" &
" " & test.Name & "\n" &
" " & funcname.astToStr & " status: " & $status & "\n" &
" " & "result: " & r.toHex() & "\n" &
" " & "expected: " & expected.toHex() & '\n'

stdout.write "Success\n"

Expand Down Expand Up @@ -99,32 +107,32 @@ proc testSha256() =

testSha256()

runPrecompileTests("modexp.json", eth_evm_modexp)
runPrecompileTests("modexp_eip2565.json", eth_evm_modexp)
runPrecompileTests("modexp.json", eth_evm_modexp, 0)
runPrecompileTests("modexp_eip2565.json", eth_evm_modexp, 0)

runPrecompileTests("bn256Add.json", eth_evm_bn254_g1add)
runPrecompileTests("bn256ScalarMul.json", eth_evm_bn254_g1mul)
runPrecompileTests("bn256Pairing.json", eth_evm_bn254_ecpairingcheck)
runPrecompileTests("bn256Add.json", eth_evm_bn254_g1add, 64)
runPrecompileTests("bn256ScalarMul.json", eth_evm_bn254_g1mul, 64)
runPrecompileTests("bn256Pairing.json", eth_evm_bn254_ecpairingcheck, 32)

runPrecompileTests("eip-2537/add_G1_bls.json", eth_evm_bls12381_g1add)
runPrecompileTests("eip-2537/fail-add_G1_bls.json", eth_evm_bls12381_g1add)
runPrecompileTests("eip-2537/add_G2_bls.json", eth_evm_bls12381_g2add)
runPrecompileTests("eip-2537/fail-add_G2_bls.json", eth_evm_bls12381_g2add)
runPrecompileTests("eip-2537/add_G1_bls.json", eth_evm_bls12381_g1add, 128)
runPrecompileTests("eip-2537/fail-add_G1_bls.json", eth_evm_bls12381_g1add, 128)
runPrecompileTests("eip-2537/add_G2_bls.json", eth_evm_bls12381_g2add, 256)
runPrecompileTests("eip-2537/fail-add_G2_bls.json", eth_evm_bls12381_g2add, 256)

runPrecompileTests("eip-2537/mul_G1_bls.json", eth_evm_bls12381_g1mul)
runPrecompileTests("eip-2537/fail-mul_G1_bls.json", eth_evm_bls12381_g1mul)
runPrecompileTests("eip-2537/mul_G2_bls.json", eth_evm_bls12381_g2mul)
runPrecompileTests("eip-2537/fail-mul_G2_bls.json", eth_evm_bls12381_g2mul)
runPrecompileTests("eip-2537/mul_G1_bls.json", eth_evm_bls12381_g1mul, 128)
runPrecompileTests("eip-2537/fail-mul_G1_bls.json", eth_evm_bls12381_g1mul, 128)
runPrecompileTests("eip-2537/mul_G2_bls.json", eth_evm_bls12381_g2mul, 256)
runPrecompileTests("eip-2537/fail-mul_G2_bls.json", eth_evm_bls12381_g2mul, 256)

runPrecompileTests("eip-2537/multiexp_G1_bls.json", eth_evm_bls12381_g1msm)
runPrecompileTests("eip-2537/fail-multiexp_G1_bls.json", eth_evm_bls12381_g1msm)
runPrecompileTests("eip-2537/multiexp_G2_bls.json", eth_evm_bls12381_g2msm)
runPrecompileTests("eip-2537/fail-multiexp_G2_bls.json", eth_evm_bls12381_g2msm)
runPrecompileTests("eip-2537/multiexp_G1_bls.json", eth_evm_bls12381_g1msm, 128)
runPrecompileTests("eip-2537/fail-multiexp_G1_bls.json", eth_evm_bls12381_g1msm, 128)
runPrecompileTests("eip-2537/multiexp_G2_bls.json", eth_evm_bls12381_g2msm, 256)
runPrecompileTests("eip-2537/fail-multiexp_G2_bls.json", eth_evm_bls12381_g2msm, 256)

runPrecompileTests("eip-2537/pairing_check_bls.json", eth_evm_bls12381_pairingcheck)
runPrecompileTests("eip-2537/fail-pairing_check_bls.json", eth_evm_bls12381_pairingcheck)
runPrecompileTests("eip-2537/pairing_check_bls.json", eth_evm_bls12381_pairingcheck, 32)
runPrecompileTests("eip-2537/fail-pairing_check_bls.json", eth_evm_bls12381_pairingcheck, 32)

runPrecompileTests("eip-2537/map_fp_to_G1_bls.json", eth_evm_bls12381_map_fp_to_g1)
runPrecompileTests("eip-2537/fail-map_fp_to_G1_bls.json", eth_evm_bls12381_map_fp_to_g1)
runPrecompileTests("eip-2537/map_fp2_to_G2_bls.json", eth_evm_bls12381_map_fp2_to_g2)
runPrecompileTests("eip-2537/fail-map_fp2_to_G2_bls.json", eth_evm_bls12381_map_fp2_to_g2)
runPrecompileTests("eip-2537/map_fp_to_G1_bls.json", eth_evm_bls12381_map_fp_to_g1, 128)
runPrecompileTests("eip-2537/fail-map_fp_to_G1_bls.json", eth_evm_bls12381_map_fp_to_g1, 128)
runPrecompileTests("eip-2537/map_fp2_to_G2_bls.json", eth_evm_bls12381_map_fp2_to_g2, 256)
runPrecompileTests("eip-2537/fail-map_fp2_to_G2_bls.json", eth_evm_bls12381_map_fp2_to_g2, 256)

0 comments on commit ad325cf

Please sign in to comment.