Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate s <= N//2 when recovering tx sender #206

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion cairo/src/utils/transaction.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, HashBuiltin, K
from starkware.cairo.common.math_cmp import is_not_zero, is_nn
from starkware.cairo.common.math import assert_not_zero, assert_nn
from starkware.cairo.common.memcpy import memcpy
from starkware.cairo.common.uint256 import Uint256
from starkware.cairo.common.uint256 import Uint256, uint256_lt

from src.model import model
from src.constants import Constants
Expand All @@ -20,6 +20,9 @@ const TX_CREATE_COST = 32000;
const TX_ACCESS_LIST_ADDRESS_COST = 2400;
const TX_ACCESS_LIST_STORAGE_KEY_COST = 1900;

const SECP256K1N_DIV_2_LOW = 0x5d576e7357a4501ddfe92f46681b20a0;
const SECP256K1N_DIV_2_HIGH = 0x7fffffffffffffffffffffffffffffff;

// @title Transaction utils
// @notice This file contains utils for decoding eth transactions
// @custom:namespace Transaction
Expand Down Expand Up @@ -396,6 +399,16 @@ namespace Transaction {
}
let range_check_ptr = [ap - 1];

// Signature validation
// `verify_eth_signature_uint256` verifies that r and s are in the range [1, N[
// TX validation imposes s to be the range [1, N//2], see EIP-2
let (is_invalid_upper_s) = uint256_lt(
Uint256(SECP256K1N_DIV_2_LOW, SECP256K1N_DIV_2_HIGH), s
);
with_attr error_message("Invalid s value") {
assert is_invalid_upper_s = FALSE;
}

let msg_hash = keccak(tx.rlp_len, tx.rlp);

Signature.verify_eth_signature_uint256(
Expand Down
29 changes: 29 additions & 0 deletions cairo/tests/programs/test_os.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
from eth_abi import encode
from hypothesis import given
from hypothesis.strategies import integers

from ethereum.crypto.elliptic_curve import SECP256K1N
from ethereum.crypto.hash import keccak256
from src.utils.uint256 import int_to_uint256
from tests.utils.constants import COINBASE, OTHER, OWNER
from tests.utils.data import block
from tests.utils.errors import cairo_error
from tests.utils.models import State
from tests.utils.solidity import get_contract

Expand Down Expand Up @@ -121,3 +126,27 @@ def test_block_hint(self, cairo_run):
else []
),
]

@given(s_value=integers(min_value=SECP256K1N // 2 + 1, max_value=SECP256K1N))
def test_should_raise_on_invalid_s_value(self, cairo_run, s_value):
initial_state = {
OWNER: {
"code": [],
"storage": {},
"balance": int(1e18),
"nonce": 0,
}
}
transactions = [{"to": OTHER, "data": "0x6001", "value": 0, "signer": OWNER}]

low, high = int_to_uint256(s_value)
block_to_pass = block(transactions)
block_to_pass.transactions[0].signature[2] = low
block_to_pass.transactions[0].signature[3] = high

with cairo_error("Invalid s value"):
cairo_run(
"test_os",
block=block_to_pass,
state=State.model_validate(initial_state),
)
Loading