Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Commit

Permalink
feat: validate signature values (#1331)
Browse files Browse the repository at this point in the history
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR:

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #1325 

## What is the new behavior?

Signature values are checked to be < 2**128

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/1331)
<!-- Reviewable:end -->
  • Loading branch information
obatirou authored Aug 8, 2024
1 parent 7112b36 commit 7fb90d7
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
10 changes: 10 additions & 0 deletions src/kakarot/accounts/library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,16 @@ namespace AccountContract {
with_attr error_message("Incorrect signature length") {
assert signature_len = 5;
}

with_attr error_message("Signatures values not in range") {
assert [range_check_ptr] = signature[0];
assert [range_check_ptr + 1] = signature[1];
assert [range_check_ptr + 2] = signature[2];
assert [range_check_ptr + 3] = signature[3];
assert [range_check_ptr + 4] = signature[4];
let range_check_ptr = range_check_ptr + 5;
}

let r = Uint256(signature[0], signature[1]);
let s = Uint256(signature[2], signature[3]);
let v = signature[4];
Expand Down
28 changes: 27 additions & 1 deletion tests/src/kakarot/accounts/test_account_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from eth_account.account import Account
from eth_utils import keccak
from hypothesis import assume, given, settings
from hypothesis.strategies import binary, composite, integers
from hypothesis.strategies import binary, composite, integers, lists, permutations
from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME
from starkware.starknet.public.abi import (
get_selector_from_name,
Expand Down Expand Up @@ -367,6 +367,32 @@ def test_should_raise_with_wrong_signature(self, cairo_run):
chain_id=CHAIN_ID,
)

@composite
def draw_signature_not_in_range(draw):
# create signature with 4 elements < 2 ** 128 and one > 2 ** 128
signature = draw(
lists(
integers(min_value=0, max_value=2**128 - 1), min_size=4, max_size=4
)
)
signature.append(
draw(integers(min_value=2**128, max_value=DEFAULT_PRIME - 1))
)
# Draw randomly signature elements
return draw(permutations(signature))

@given(draw_signature_not_in_range())
def test_should_raise_with_signature_values_not_in_range(
self, cairo_run, draw_signature_not_in_range
):
with cairo_error(message="Signatures values not in range"):
cairo_run(
"test__execute_from_outside",
tx_data=[1],
signature=draw_signature_not_in_range,
chain_id=CHAIN_ID,
)

@SyscallHandler.patch("Account_evm_address", int(ARACHNID_PROXY_DEPLOYER, 16))
def test_should_raise_unauthorized_pre_eip155_tx(self, cairo_run):
rlp_decoded = rlp.decode(ARACHNID_PROXY_SIGNED_TX)
Expand Down

0 comments on commit 7fb90d7

Please sign in to comment.