From 96829dc07f39b7d464cb3b9d22e25712abc9b72b Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:17:49 +0100 Subject: [PATCH] fix: ecrecover validation + migrate to KeccakBuiltin-based ecrecover (#88) Fixes the ecrecover validatoin issues raised in audit + migrate ecrecover to use the KeccakBuiltin. --- .github/workflows/python_tests.yml | 5 +- .gitignore | 3 + .python-version | 1 - .vscode/settings.json | 6 +- cairo/src/interpreter.cairo | 17 ++++- cairo/src/precompiles/blake2f.cairo | 14 ++-- cairo/src/precompiles/datacopy.cairo | 13 ++-- cairo/src/precompiles/ec_recover.cairo | 72 ++++++++++++++----- cairo/src/precompiles/ecadd.cairo | 13 ++-- cairo/src/precompiles/ecmul.cairo | 13 ++-- .../src/precompiles/kakarot_precompiles.cairo | 7 +- cairo/src/precompiles/modexp.cairo | 13 ++-- cairo/src/precompiles/p256verify.cairo | 13 ++-- cairo/src/precompiles/precompiles.cairo | 32 +++++++-- cairo/src/precompiles/ripemd160.cairo | 13 ++-- cairo/src/precompiles/sha256.cairo | 13 ++-- cairo/src/utils/signature.cairo | 50 ++++++++++++- cairo/tests/conftest.py | 30 ++++++++ .../tests/src/precompiles/test_datacopy.cairo | 10 ++- cairo/tests/src/precompiles/test_ec_add.cairo | 9 ++- cairo/tests/src/precompiles/test_ec_mul.cairo | 9 ++- .../src/precompiles/test_ec_recover.cairo | 10 ++- .../tests/src/precompiles/test_ec_recover.py | 59 +++++++++++---- cairo/tests/src/precompiles/test_modexp.cairo | 11 +-- .../src/precompiles/test_ripemd160.cairo | 11 +-- cairo/tests/src/precompiles/test_sha256.cairo | 11 +-- uv.lock | 2 - 27 files changed, 355 insertions(+), 105 deletions(-) delete mode 100644 .python-version diff --git a/.github/workflows/python_tests.yml b/.github/workflows/python_tests.yml index 7b82c957..d5537f1c 100644 --- a/.github/workflows/python_tests.yml +++ b/.github/workflows/python_tests.yml @@ -13,13 +13,10 @@ jobs: HYPOTHESIS_PROFILE: ci steps: - uses: actions/checkout@v4 - - uses: astral-sh/setup-uv@v2 + - uses: astral-sh/setup-uv@v3 with: enable-cache: true cache-dependency-glob: uv.lock - - uses: actions/setup-python@v5 - with: - python-version-file: .python-version - run: | cd cairo uv run compile diff --git a/.gitignore b/.gitignore index 14580aac..af4bfed5 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,6 @@ cairo/tests/ef_tests/test_data # Ignore proptest regression files **/proptest-regressions/* + +# Ide settings +.vscode/ diff --git a/.python-version b/.python-version deleted file mode 100644 index 9919bf8c..00000000 --- a/.python-version +++ /dev/null @@ -1 +0,0 @@ -3.10.13 diff --git a/.vscode/settings.json b/.vscode/settings.json index 09ac58de..cbabc031 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -12,5 +12,9 @@ "python.testing.unittestEnabled": false, "python.testing.pytestEnabled": true, "jupyter.notebookFileRoot": "${workspaceFolder}/cairo", - "githubPullRequests.remotes": ["kakarot", "upstream"] + "githubPullRequests.remotes": [ + "kakarot", + "upstream" + ], + "cairols.sourceDir": "cairo" } diff --git a/cairo/src/interpreter.cairo b/cairo/src/interpreter.cairo index a7f8a62a..100595e9 100644 --- a/cairo/src/interpreter.cairo +++ b/cairo/src/interpreter.cairo @@ -1,6 +1,6 @@ from starkware.cairo.common.alloc import alloc from starkware.cairo.common.bool import FALSE, TRUE -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math_cmp import is_not_zero, is_nn, is_le_felt from starkware.cairo.common.math import split_felt from starkware.cairo.common.default_dict import default_dict_new @@ -45,6 +45,7 @@ namespace Interpreter { pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, stack: model.Stack*, memory: model.Memory*, state: model.State*, @@ -693,6 +694,9 @@ namespace Interpreter { let evm = cast([ap - 1], model.EVM*); let evm_prev = cast([fp - 3], model.EVM*); + // keccak is still unused in regular opcode execution + let keccak_ptr = cast([fp - 7], KeccakBuiltin*); + if (evm_prev.message.depth == evm.message.depth) { let evm = EVM.increment_program_counter(evm, 1); return evm; @@ -709,6 +713,9 @@ namespace Interpreter { let state = cast([ap - 2], model.State*); let evm = cast([ap - 1], model.EVM*); + // keccak is still unused in regular opcode execution + let keccak_ptr = cast([fp - 7], KeccakBuiltin*); + return evm; } @@ -735,6 +742,7 @@ namespace Interpreter { pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, stack: model.Stack*, memory: model.Memory*, state: model.State*, @@ -811,7 +819,12 @@ namespace Interpreter { // @return gas_used the gas used by the transaction // @return required_gas The amount of gas required by the transaction to successfully execute. This is different // from the gas used by the transaction as it doesn't take into account any refunds. - func execute{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( + func execute{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }( env: model.Environment*, address: felt, is_deploy_tx: felt, diff --git a/cairo/src/precompiles/blake2f.cairo b/cairo/src/precompiles/blake2f.cairo index 2c9e624d..7d399f84 100644 --- a/cairo/src/precompiles/blake2f.cairo +++ b/cairo/src/precompiles/blake2f.cairo @@ -1,6 +1,5 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin -from starkware.cairo.common.cairo_builtins import BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.registers import get_fp_and_pc, get_label_location from starkware.cairo.common.math_cmp import is_nn from starkware.cairo.common.bool import FALSE @@ -23,9 +22,14 @@ namespace PrecompileBlake2f { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; local rounds_bytes_len = 4; local word_bytes_len = 8; diff --git a/cairo/src/precompiles/datacopy.cairo b/cairo/src/precompiles/datacopy.cairo index 220f6df6..31886915 100644 --- a/cairo/src/precompiles/datacopy.cairo +++ b/cairo/src/precompiles/datacopy.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from src.model import model from src.utils.utils import Helpers @@ -19,9 +19,14 @@ namespace PrecompileDataCopy { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { let (minimum_word_size) = Helpers.minimum_word_count(input_len); return (input_len, input, 3 * minimum_word_size + GAS_COST_DATACOPY, 0); } diff --git a/cairo/src/precompiles/ec_recover.cairo b/cairo/src/precompiles/ec_recover.cairo index 59cf1e84..4a5d5b40 100644 --- a/cairo/src/precompiles/ec_recover.cairo +++ b/cairo/src/precompiles/ec_recover.cairo @@ -1,22 +1,20 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.builtin_keccak.keccak import keccak_uint256s_bigend from starkware.cairo.common.bool import FALSE from starkware.cairo.common.math_cmp import RC_BOUND from starkware.cairo.common.cairo_secp.ec import EcPoint from starkware.cairo.common.cairo_secp.bigint import BigInt3 -from starkware.cairo.common.cairo_secp.signature import ( - recover_public_key, - public_key_point_to_eth_address, -) -from starkware.cairo.common.uint256 import Uint256, uint256_reverse_endian -from starkware.cairo.common.cairo_secp.bigint import bigint_to_uint256 +from starkware.cairo.common.cairo_secp.signature import recover_public_key +from starkware.cairo.common.uint256 import Uint256, uint256_reverse_endian, uint256_lt +from starkware.cairo.common.cairo_secp.bigint import bigint_to_uint256, uint256_to_bigint from starkware.cairo.common.memset import memset from src.errors import Errors -from src.interfaces.interfaces import ICairo1Helpers +from src.utils.uint256 import uint256_eq from src.utils.utils import Helpers from src.utils.array import slice +from src.utils.signature import Signature from src.utils.maths import unsigned_div_rem // @title EcRecover Precompile related functions. @@ -28,15 +26,23 @@ namespace PrecompileEcRecover { const PRECOMPILE_ADDRESS = 0x01; const GAS_COST_EC_RECOVER = 3000; + const SECP256K1N_HIGH = 0xfffffffffffffffffffffffffffffffe; + const SECP256K1N_LOW = 0xbaaedce6af48a03bbfd25e8cd0364141; + // @notice Run the precompile. // @param input_len The length of input array. // @param input The input array. // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let (input_padded) = alloc(); @@ -50,19 +56,47 @@ namespace PrecompileEcRecover { return (0, output, GAS_COST_EC_RECOVER, 0); } - let msg_hash = Helpers.bytes_to_uint256(32, input_padded); + let msg_hash_bigint = Helpers.bytes32_to_bigint(input_padded); let r = Helpers.bytes_to_uint256(32, input_padded + 32 * 2); let s = Helpers.bytes_to_uint256(32, input_padded + 32 * 3); - // v - 27, see recover_public_key comment - let (success, recovered_address) = ICairo1Helpers.recover_eth_address( - msg_hash=msg_hash, r=r, s=s, y_parity=v - 27 - ); + let SECP256K1N = Uint256(low=SECP256K1N_LOW, high=SECP256K1N_HIGH); + let (is_valid_upper_r) = uint256_lt(r, SECP256K1N); + let (is_valid_upper_s) = uint256_lt(s, SECP256K1N); + let is_valid_upper_bound = is_valid_upper_r * is_valid_upper_s; + if (is_valid_upper_bound == FALSE) { + let (output) = alloc(); + return (0, output, GAS_COST_EC_RECOVER, 0); + } + + let (is_invalid_lower_r) = uint256_eq(r, Uint256(low=0, high=0)); + let (is_invalid_lower_s) = uint256_eq(s, Uint256(low=0, high=0)); + let is_invalid_lower_bound = is_invalid_lower_r + is_invalid_lower_s; + if (is_invalid_lower_bound != FALSE) { + let (output) = alloc(); + return (0, output, GAS_COST_EC_RECOVER, 0); + } + let (r_bigint) = uint256_to_bigint(r); + let (s_bigint) = uint256_to_bigint(s); + let (public_key_point, success) = Signature.try_recover_public_key( + msg_hash_bigint, r_bigint, s_bigint, v - 27 + ); if (success == 0) { let (output) = alloc(); return (0, output, GAS_COST_EC_RECOVER, 0); } + let (is_public_key_invalid) = EcRecoverHelpers.ec_point_equal( + public_key_point, EcPoint(BigInt3(0, 0, 0), BigInt3(0, 0, 0)) + ); + if (is_public_key_invalid != 0) { + let (output) = alloc(); + return (0, output, GAS_COST_EC_RECOVER, 0); + } + + let (recovered_address) = EcRecoverHelpers.public_key_point_to_eth_address( + public_key_point + ); let (output) = alloc(); memset(output, 0, 12); @@ -83,8 +117,12 @@ namespace EcRecoverHelpers { } // @notice Convert a public key point to the corresponding Ethereum address. + // @dev Uses the `KeccakBuiltin` builtin, while the one in Starkware's CairoZero library does not. func public_key_point_to_eth_address{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }(public_key_point: EcPoint) -> (eth_address: felt) { alloc_locals; let (local elements: Uint256*) = alloc(); diff --git a/cairo/src/precompiles/ecadd.cairo b/cairo/src/precompiles/ecadd.cairo index a436d678..e6bc078e 100644 --- a/cairo/src/precompiles/ecadd.cairo +++ b/cairo/src/precompiles/ecadd.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.cairo_secp.bigint import BigInt3 from starkware.cairo.common.uint256 import Uint256 from starkware.cairo.common.memcpy import memcpy @@ -22,9 +22,14 @@ namespace PrecompileEcAdd { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let x0: BigInt3 = Helpers.bytes32_to_bigint(input); diff --git a/cairo/src/precompiles/ecmul.cairo b/cairo/src/precompiles/ecmul.cairo index 3b89b678..ffc1ca24 100644 --- a/cairo/src/precompiles/ecmul.cairo +++ b/cairo/src/precompiles/ecmul.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.cairo_secp.bigint import BigInt3 from starkware.cairo.common.uint256 import Uint256 from starkware.cairo.common.memcpy import memcpy @@ -22,9 +22,14 @@ namespace PrecompileEcMul { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let x: BigInt3 = Helpers.bytes32_to_bigint(input); diff --git a/cairo/src/precompiles/kakarot_precompiles.cairo b/cairo/src/precompiles/kakarot_precompiles.cairo index 644c92eb..68a7d44d 100644 --- a/cairo/src/precompiles/kakarot_precompiles.cairo +++ b/cairo/src/precompiles/kakarot_precompiles.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math_cmp import is_nn from starkware.cairo.common.math import assert_not_zero from starkware.cairo.common.alloc import alloc @@ -20,7 +20,10 @@ namespace KakarotPrecompiles { // @param input The input data. // @param caller_address The address of the caller of the precompile. Delegatecall rules apply. func cairo_precompile{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }(input_len: felt, input: felt*, caller_address: felt) -> ( output_len: felt, output: felt*, gas_used: felt, reverted: felt ) { diff --git a/cairo/src/precompiles/modexp.cairo b/cairo/src/precompiles/modexp.cairo index a794a5b6..7b06965d 100644 --- a/cairo/src/precompiles/modexp.cairo +++ b/cairo/src/precompiles/modexp.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.uint256 import Uint256 from starkware.cairo.common.alloc import alloc @@ -20,9 +20,14 @@ namespace PrecompileModExpUint256 { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let b_size: Uint256 = Helpers.bytes32_to_uint256(input); diff --git a/cairo/src/precompiles/p256verify.cairo b/cairo/src/precompiles/p256verify.cairo index 0eb4b84e..1df4d6f9 100644 --- a/cairo/src/precompiles/p256verify.cairo +++ b/cairo/src/precompiles/p256verify.cairo @@ -1,5 +1,5 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.memset import memset from src.utils.utils import Helpers @@ -20,9 +20,14 @@ namespace PrecompileP256Verify { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let (output) = alloc(); diff --git a/cairo/src/precompiles/precompiles.cairo b/cairo/src/precompiles/precompiles.cairo index afd525a4..bc43c13c 100644 --- a/cairo/src/precompiles/precompiles.cairo +++ b/cairo/src/precompiles/precompiles.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math_cmp import is_nn, is_not_zero, is_in_range from starkware.cairo.common.alloc import alloc from starkware.cairo.common.bool import FALSE @@ -34,7 +34,12 @@ namespace Precompiles { // @return output The output array. // @return gas_used The gas usage of precompile. // @return reverted Whether the precompile ran successfully or not - func exec_precompile{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( + func exec_precompile{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }( precompile_address: felt, input_len: felt, input: felt*, @@ -72,6 +77,7 @@ namespace Precompiles { [ap] = pedersen_ptr, ap++; [ap] = range_check_ptr, ap++; [ap] = bitwise_ptr, ap++; + [ap] = keccak_ptr, ap++; call unauthorized_precompile; ret; @@ -84,6 +90,7 @@ namespace Precompiles { [ap] = pedersen_ptr, ap++; [ap] = range_check_ptr, ap++; [ap] = bitwise_ptr, ap++; + [ap] = keccak_ptr, ap++; [ap] = precompile_address, ap++; [ap] = input_len, ap++; [ap] = input, ap++; @@ -125,6 +132,7 @@ namespace Precompiles { [ap] = pedersen_ptr, ap++; [ap] = range_check_ptr, ap++; [ap] = bitwise_ptr, ap++; + [ap] = keccak_ptr, ap++; [ap] = input_len, ap++; [ap] = input, ap++; [ap] = caller_address, ap++; @@ -142,7 +150,10 @@ namespace Precompiles { // @param input_len The length of the input array. // @param input The input array. func unauthorized_precompile{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }() -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { let (revert_reason_len, revert_reason) = Errors.unauthorizedPrecompile(); return (revert_reason_len, revert_reason, 0, Errors.REVERT); @@ -154,7 +165,10 @@ namespace Precompiles { // @param input_len The length of the input array. // @param input The input array. func unknown_precompile{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }(evm_address: felt, input_len: felt, input: felt*) -> ( output_len: felt, output: felt*, gas_used: felt, reverted: felt ) { @@ -168,7 +182,10 @@ namespace Precompiles { // @param input_len The length of the input array. // @param input The input array. func not_implemented_precompile{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }(evm_address: felt, input_len: felt, input: felt*) -> ( output_len: felt, output: felt*, gas_used: felt, reverted: felt ) { @@ -177,7 +194,10 @@ namespace Precompiles { } func external_precompile{ - pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, }(evm_address: felt, input_len: felt, input: felt*) -> ( output_len: felt, output: felt*, gas_used: felt, reverted: felt ) { diff --git a/cairo/src/precompiles/ripemd160.cairo b/cairo/src/precompiles/ripemd160.cairo index 7803f058..f5dab097 100644 --- a/cairo/src/precompiles/ripemd160.cairo +++ b/cairo/src/precompiles/ripemd160.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, HashBuiltin +from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, HashBuiltin, KeccakBuiltin from starkware.cairo.common.alloc import alloc from starkware.cairo.common.math import assert_nn_le from starkware.cairo.common.math_cmp import is_nn_le, is_nn @@ -32,9 +32,14 @@ namespace PrecompileRIPEMD160 { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; let (local buf: felt*) = alloc(); let (local arr_x: felt*) = alloc(); diff --git a/cairo/src/precompiles/sha256.cairo b/cairo/src/precompiles/sha256.cairo index 76220116..08d3abf8 100644 --- a/cairo/src/precompiles/sha256.cairo +++ b/cairo/src/precompiles/sha256.cairo @@ -1,6 +1,6 @@ from starkware.cairo.common.alloc import alloc from starkware.cairo.common.registers import get_fp_and_pc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math import assert_nn_le from starkware.cairo.common.math_cmp import is_le_felt from starkware.cairo.common.memcpy import memcpy @@ -31,9 +31,14 @@ namespace PrecompileSHA256 { // @return output_len The output length. // @return output The output array. // @return gas_used The gas usage of precompile. - func run{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - _address: felt, input_len: felt, input: felt* - ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { + func run{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, + }(_address: felt, input_len: felt, input: felt*) -> ( + output_len: felt, output: felt*, gas_used: felt, reverted: felt + ) { alloc_locals; // Copy input array diff --git a/cairo/src/utils/signature.cairo b/cairo/src/utils/signature.cairo index a409e207..4b943688 100644 --- a/cairo/src/utils/signature.cairo +++ b/cairo/src/utils/signature.cairo @@ -1,7 +1,15 @@ from starkware.cairo.common.cairo_builtins import BitwiseBuiltin from starkware.cairo.common.cairo_secp.bigint3 import BigInt3, UnreducedBigInt3 -from starkware.cairo.common.cairo_secp.signature import validate_signature_entry +from starkware.cairo.common.cairo_secp.ec_point import EcPoint +from starkware.cairo.common.cairo_secp.signature import ( + validate_signature_entry, + try_get_point_from_x, + get_generator_point, + div_mod_n, +) +from starkware.cairo.common.cairo_secp.ec import ec_add, ec_mul, ec_negate from starkware.cairo.common.uint256 import Uint256 +from starkware.cairo.common.alloc import alloc from starkware.cairo.common.cairo_secp.bigint import uint256_to_bigint from src.interfaces.interfaces import ICairo1Helpers @@ -32,4 +40,44 @@ namespace Signature { } return (); } + + // Similar to `recover_public_key`, but handles the case where 'x' does not correspond to a point on the + // curve gracefully. + // Receives a signature and the signed message hash. + // Returns the public key associated with the signer, represented as a point on the curve, and `true` if valid. + // Returns the point (0, 0) and `false` otherwise. + // Note: + // Some places use the values 27 and 28 instead of 0 and 1 for v. + // In that case, a subtraction by 27 returns a v that can be used by this function. + // Prover assumptions: + // * r is the x coordinate of some nonzero point on the curve. + // * All the limbs of s and msg_hash are in the range (-2 ** 210.99, 2 ** 210.99). + // * All the limbs of r are in the range (-2 ** 124.99, 2 ** 124.99). + func try_recover_public_key{range_check_ptr}( + msg_hash: BigInt3, r: BigInt3, s: BigInt3, v: felt + ) -> (public_key_point: EcPoint, success: felt) { + alloc_locals; + let (local r_point: EcPoint*) = alloc(); + let (is_on_curve) = try_get_point_from_x(x=r, v=v, result=r_point); + if (is_on_curve == 0) { + return (public_key_point=EcPoint(x=BigInt3(0, 0, 0), y=BigInt3(0, 0, 0)), success=0); + } + let (generator_point: EcPoint) = get_generator_point(); + // The result is given by + // -(msg_hash / r) * gen + (s / r) * r_point + // where the division by r is modulo N. + + let (u1: BigInt3) = div_mod_n(msg_hash, r); + let (u2: BigInt3) = div_mod_n(s, r); + + let (point1) = ec_mul(generator_point, u1); + // We prefer negating the point over negating the scalar because negating mod SECP_P is + // computationally easier than mod N. + let (minus_point1) = ec_negate(point1); + + let (point2) = ec_mul([r_point], u2); + + let (public_key_point) = ec_add(minus_point1, point2); + return (public_key_point=public_key_point, success=1); + } } diff --git a/cairo/tests/conftest.py b/cairo/tests/conftest.py index d5919e96..eac1787e 100644 --- a/cairo/tests/conftest.py +++ b/cairo/tests/conftest.py @@ -1,9 +1,11 @@ import logging +import os import shutil from pathlib import Path import pytest from dotenv import load_dotenv +from hypothesis import Phase, Verbosity, settings from tests.utils.coverage import report_runs from tests.utils.reporting import dump_coverage @@ -35,3 +37,31 @@ async def coverage(worker_id): output_dir.mkdir(exist_ok=True, parents=True) shutil.rmtree(output_dir, ignore_errors=True) dump_coverage(output_dir, files) + + +settings.register_profile( + "nightly", + deadline=None, + max_examples=1500, + phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target], +) +settings.register_profile( + "ci", + deadline=None, + max_examples=100, + phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target], +) +settings.register_profile( + "dev", + deadline=None, + max_examples=10, + phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target], +) +settings.register_profile( + "debug", + max_examples=10, + verbosity=Verbosity.verbose, + phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target], +) +settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default")) +logger.info(f"Using Hypothesis profile: {os.getenv('HYPOTHESIS_PROFILE', 'default')}") diff --git a/cairo/tests/src/precompiles/test_datacopy.cairo b/cairo/tests/src/precompiles/test_datacopy.cairo index 826416eb..2c7b5049 100644 --- a/cairo/tests/src/precompiles/test_datacopy.cairo +++ b/cairo/tests/src/precompiles/test_datacopy.cairo @@ -1,12 +1,16 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.alloc import alloc from src.utils.utils import Helpers from src.precompiles.datacopy import PrecompileDataCopy from tests.utils.helpers import TestHelpers -func test__datacopy_impl{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - ) { +func test__datacopy_impl{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}() { // Given # 1 alloc_locals; local calldata_len: felt; diff --git a/cairo/tests/src/precompiles/test_ec_add.cairo b/cairo/tests/src/precompiles/test_ec_add.cairo index 741c8570..7e0e93be 100644 --- a/cairo/tests/src/precompiles/test_ec_add.cairo +++ b/cairo/tests/src/precompiles/test_ec_add.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.cairo_secp.bigint import BigInt3, bigint_to_uint256, uint256_to_bigint from starkware.cairo.common.uint256 import Uint256, assert_uint256_eq from starkware.cairo.common.math import split_felt @@ -19,7 +19,12 @@ from tests.utils.helpers import TestHelpers const G1POINT_BYTES_LEN = 32; -func test__ecadd_impl{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}() { +func test__ecadd_impl{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}() { // Given alloc_locals; local calldata_len: felt; diff --git a/cairo/tests/src/precompiles/test_ec_mul.cairo b/cairo/tests/src/precompiles/test_ec_mul.cairo index 738a340c..ad0242d1 100644 --- a/cairo/tests/src/precompiles/test_ec_mul.cairo +++ b/cairo/tests/src/precompiles/test_ec_mul.cairo @@ -1,5 +1,5 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.cairo_secp.bigint import BigInt3, bigint_to_uint256, uint256_to_bigint from starkware.cairo.common.uint256 import Uint256 from starkware.cairo.common.memcpy import memcpy @@ -11,7 +11,12 @@ from tests.utils.helpers import TestHelpers const G1POINT_BYTES_LEN = 32; -func test__ecmul_impl{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}() { +func test__ecmul_impl{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}() { // Given alloc_locals; diff --git a/cairo/tests/src/precompiles/test_ec_recover.cairo b/cairo/tests/src/precompiles/test_ec_recover.cairo index 7f09f48d..48d585d2 100644 --- a/cairo/tests/src/precompiles/test_ec_recover.cairo +++ b/cairo/tests/src/precompiles/test_ec_recover.cairo @@ -1,5 +1,5 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math import unsigned_div_rem, assert_not_zero from starkware.cairo.common.memset import memset from starkware.cairo.common.memcpy import memcpy @@ -7,8 +7,12 @@ from starkware.cairo.common.memcpy import memcpy from src.precompiles.ec_recover import PrecompileEcRecover from src.utils.utils import Helpers -func test__ec_recover{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - ) -> (output: felt*) { +func test__ec_recover{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}() -> (output: felt*) { alloc_locals; let (local input) = alloc(); tempvar input_len: felt; diff --git a/cairo/tests/src/precompiles/test_ec_recover.py b/cairo/tests/src/precompiles/test_ec_recover.py index 1d4a6757..af80c493 100644 --- a/cairo/tests/src/precompiles/test_ec_recover.py +++ b/cairo/tests/src/precompiles/test_ec_recover.py @@ -3,6 +3,8 @@ from ethereum.crypto.elliptic_curve import SECP256K1N, secp256k1_recover from ethereum.crypto.hash import Hash32, keccak256 from ethereum.utils.byte import left_pad_zero_bytes +from hypothesis import given +from hypothesis import strategies as st from tests.utils.helpers import ec_sign, generate_random_private_key @@ -32,12 +34,13 @@ def ecrecover(data): return padded_address, public_key -@pytest.mark.xfail(reason="Cairo1Helpers migration") @pytest.mark.EC_RECOVER class TestEcRecover: - def test_should_return_evm_address_in_bytes32(self, cairo_run): + @given(message=st.binary(min_size=1, max_size=1000)) + def test_valid_signature(self, message, cairo_run): + """Test with valid signatures generated from random messages.""" private_key = generate_random_private_key() - msg = keccak256(b"test message") + msg = keccak256(message) (v, r, s) = ec_sign(msg, private_key) input_data = [ @@ -48,25 +51,53 @@ def test_should_return_evm_address_in_bytes32(self, cairo_run): ] padded_address, _ = ecrecover(input_data) - [output] = cairo_run("test__ec_recover", input=input_data) - assert bytes(output) == bytes(padded_address) - def test_should_fail_when_input_len_is_not_128(self, cairo_run): - [output] = cairo_run("test__ec_recover", input=[]) + @given(input_length=st.integers(min_value=0, max_value=127)) + def test_invalid_input_length(self, input_length, cairo_run): + """Test with various invalid input lengths.""" + input_data = [0] * input_length + [output] = cairo_run("test__ec_recover", input=input_data) assert output == [] - def test_should_fail_when_recovery_identifier_is_neither_27_nor_28(self, cairo_run): - private_key = generate_random_private_key() - msg = keccak256(b"test message") - (_, r, s) = ec_sign(msg, private_key) - v = 1 + @given( + v=st.integers(min_value=0, max_value=26) | st.integers(min_value=29), + msg=st.binary(min_size=32, max_size=32), + r=st.integers(min_value=1, max_value=SECP256K1N - 1), + s=st.integers(min_value=1, max_value=SECP256K1N - 1), + ) + def test_invalid_v(self, v, msg, r, s, cairo_run): + """Test with invalid v values.""" input_data = [ *msg, *v.to_bytes(32, "big"), - *r, - *s, + *r.to_bytes(32, "big"), + *s.to_bytes(32, "big"), ] [output] = cairo_run("test__ec_recover", input=input_data) assert output == [] + + @given( + v=st.integers(min_value=27, max_value=28), + msg=st.binary(min_size=32, max_size=32), + r=st.integers(min_value=0, max_value=2**256 - 1), + s=st.integers(min_value=0, max_value=2**256 - 1), + ) + def test_parameter_boundaries(self, cairo_run, v, msg, r, s): + """Test `r` and `s` parameter validation including boundary conditions.""" + input_data = [ + *msg, + *v.to_bytes(32, "big"), + *r.to_bytes(32, "big"), + *s.to_bytes(32, "big"), + ] + + py_result = ecrecover(input_data) + [cairo_result] = cairo_run("test__ec_recover", input=input_data) + + if py_result is None: + assert cairo_result == [] + else: + py_address, _ = py_result + assert bytes(cairo_result) == bytes(py_address) diff --git a/cairo/tests/src/precompiles/test_modexp.cairo b/cairo/tests/src/precompiles/test_modexp.cairo index 671449db..0185827e 100644 --- a/cairo/tests/src/precompiles/test_modexp.cairo +++ b/cairo/tests/src/precompiles/test_modexp.cairo @@ -1,13 +1,16 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.math import split_felt from src.utils.utils import Helpers from src.precompiles.modexp import PrecompileModExpUint256 -func test__modexp_impl{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - output_ptr: felt* -) { +func test__modexp_impl{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}(output_ptr: felt*) { alloc_locals; local data_len: felt; let (data: felt*) = alloc(); diff --git a/cairo/tests/src/precompiles/test_ripemd160.cairo b/cairo/tests/src/precompiles/test_ripemd160.cairo index a60dde20..50f405c7 100644 --- a/cairo/tests/src/precompiles/test_ripemd160.cairo +++ b/cairo/tests/src/precompiles/test_ripemd160.cairo @@ -1,12 +1,15 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.memcpy import memcpy from src.precompiles.ripemd160 import PrecompileRIPEMD160 -func test__ripemd160{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - output_ptr: felt* -) { +func test__ripemd160{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}(output_ptr: felt*) { alloc_locals; tempvar msg_len: felt; let (msg: felt*) = alloc(); diff --git a/cairo/tests/src/precompiles/test_sha256.cairo b/cairo/tests/src/precompiles/test_sha256.cairo index 22ead776..7bf9a73a 100644 --- a/cairo/tests/src/precompiles/test_sha256.cairo +++ b/cairo/tests/src/precompiles/test_sha256.cairo @@ -1,13 +1,16 @@ from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin +from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin, KeccakBuiltin from starkware.cairo.common.memcpy import memcpy from src.precompiles.sha256 import PrecompileSHA256 from src.utils.utils import Helpers -func test__sha256{pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - output_ptr: felt* -) { +func test__sha256{ + pedersen_ptr: HashBuiltin*, + range_check_ptr, + bitwise_ptr: BitwiseBuiltin*, + keccak_ptr: KeccakBuiltin*, +}(output_ptr: felt*) { alloc_locals; local data_len: felt; let (data: felt*) = alloc(); diff --git a/uv.lock b/uv.lock index 9464e113..43d24eef 100644 --- a/uv.lock +++ b/uv.lock @@ -1988,8 +1988,6 @@ version = "6.0.0" source = { registry = "https://pypi.org/simple" } sdist = { url = "https://files.pythonhosted.org/packages/18/c7/8c6872f7372eb6a6b2e4708b88419fb46b857f7a2e1892966b851cc79fc9/psutil-6.0.0.tar.gz", hash = "sha256:8faae4f310b6d969fa26ca0545338b21f73c6b15db7c4a8d934a5482faa818f2", size = 508067 } wheels = [ - { url = "https://files.pythonhosted.org/packages/c5/66/78c9c3020f573c58101dc43a44f6855d01bbbd747e24da2f0c4491200ea3/psutil-6.0.0-cp27-none-win32.whl", hash = "sha256:02b69001f44cc73c1c5279d02b30a817e339ceb258ad75997325e0e6169d8b35", size = 249766 }, - { url = "https://files.pythonhosted.org/packages/e1/3f/2403aa9558bea4d3854b0e5e567bc3dd8e9fbc1fc4453c0aa9aafeb75467/psutil-6.0.0-cp27-none-win_amd64.whl", hash = "sha256:21f1fb635deccd510f69f485b87433460a603919b45e2a324ad65b0cc74f8fb1", size = 253024 }, { url = "https://files.pythonhosted.org/packages/0b/37/f8da2fbd29690b3557cca414c1949f92162981920699cd62095a984983bf/psutil-6.0.0-cp36-abi3-macosx_10_9_x86_64.whl", hash = "sha256:c588a7e9b1173b6e866756dde596fd4cad94f9399daf99ad8c3258b3cb2b47a0", size = 250961 }, { url = "https://files.pythonhosted.org/packages/35/56/72f86175e81c656a01c4401cd3b1c923f891b31fbcebe98985894176d7c9/psutil-6.0.0-cp36-abi3-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:6ed2440ada7ef7d0d608f20ad89a04ec47d2d3ab7190896cd62ca5fc4fe08bf0", size = 287478 }, { url = "https://files.pythonhosted.org/packages/19/74/f59e7e0d392bc1070e9a70e2f9190d652487ac115bb16e2eff6b22ad1d24/psutil-6.0.0-cp36-abi3-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:5fd9a97c8e94059b0ef54a7d4baf13b405011176c3b6ff257c247cae0d560ecd", size = 290455 },