Skip to content

Commit

Permalink
Make sure all uint256 are valid (#370)
Browse files Browse the repository at this point in the history
* Make sure all uint256 are valid

* Fix compilaton of felt_utils
  • Loading branch information
pscott committed Oct 20, 2022
1 parent fef974a commit c71f8f3
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 9 deletions.
2 changes: 0 additions & 2 deletions contracts/starknet/ExecutionStrategies/Vanilla.cairo
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
%lang starknet

from starkware.cairo.common.uint256 import Uint256

@external
func execute{syscall_ptr: felt*}(
proposal_outcome: felt, execution_params_len: felt, execution_params: felt*
Expand Down
1 change: 1 addition & 0 deletions contracts/starknet/SpaceAccount.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ from contracts.starknet.lib.voting import Voting
from contracts.starknet.lib.general_address import Address
from contracts.starknet.lib.proposal_info import ProposalInfo
from contracts.starknet.lib.vote import Vote
from contracts.starknet.lib.uint256_utils import Uint256Utils

//
// Constructor
Expand Down
7 changes: 3 additions & 4 deletions contracts/starknet/VotingStrategies/Whitelist.cairo
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
%lang starknet

from starkware.cairo.common.uint256 import Uint256, uint256_check
from starkware.cairo.common.uint256 import Uint256
from starkware.cairo.common.cairo_builtins import HashBuiltin
from contracts.starknet.lib.general_address import Address
from contracts.starknet.lib.uint256_utils import Uint256Utils

@storage_var
func whitelist(address: Address) -> (voting_power: Uint256) {
Expand Down Expand Up @@ -45,9 +46,7 @@ func register_whitelist{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_ch
// Add it to the whitelist
let voting_power = Uint256(_whitelist[1], _whitelist[2]);

with_attr error_message("Whitelist: Invalid uint256 for voting power") {
uint256_check(voting_power);
}
Uint256Utils.assert_valid_uint256(voting_power);

whitelist.write(address, voting_power);

Expand Down
17 changes: 17 additions & 0 deletions contracts/starknet/lib/eip712.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ from starkware.cairo.common.uint256 import (
)
from contracts.starknet.lib.felt_utils import FeltUtils
from contracts.starknet.lib.array_utils import ArrayUtils
from contracts.starknet.lib.uint256_utils import Uint256Utils

const ETHEREUM_PREFIX = 0x1901;

Expand Down Expand Up @@ -68,6 +69,10 @@ namespace EIP712 {
) {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

let voter_address = calldata[0];

let (authenticator_address) = get_contract_address();
Expand Down Expand Up @@ -165,6 +170,10 @@ namespace EIP712 {
) {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

// Proposer address should be located in calldata[0]
let proposer_address = calldata[0];

Expand Down Expand Up @@ -283,6 +292,10 @@ namespace EIP712 {
) -> () {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

// Ensure user has not already used this salt in a previous action
let (already_used) = EIP712_salts.read(eth_address, salt);
with_attr error_message("EIP712: Salt already used") {
Expand Down Expand Up @@ -352,6 +365,10 @@ namespace EIP712 {
) -> () {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

// Ensure user has not already used this salt in a previous action
let (already_used) = EIP712_salts.read(eth_address, salt);
with_attr error_message("EIP712: Salt already used") {
Expand Down
7 changes: 6 additions & 1 deletion contracts/starknet/lib/felt_utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ from starkware.cairo.common.cairo_builtins import BitwiseBuiltin
from starkware.cairo.common.math import unsigned_div_rem, split_felt, assert_nn_le
from starkware.cairo.common.bitwise import bitwise_and
from starkware.cairo.common.uint256 import Uint256
from contracts.starknet.lib.uint256_utils import Uint256Utils

const MAX_32 = 2 ** 32 - 1;

Expand All @@ -22,7 +23,11 @@ namespace FeltUtils {
) {
let word1_shifted = word1 * SHIFT_64;
let word3_shifted = word3 * SHIFT_64;
return (Uint256(low=word3_shifted + word4, high=word1_shifted + word2),);
let result = Uint256(low=word3_shifted + word4, high=word1_shifted + word2);

Uint256Utils.assert_valid_uint256(result);

return (result,);
}

// Converts a felt to a Uint256.
Expand Down
11 changes: 11 additions & 0 deletions contracts/starknet/lib/session_key.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ from contracts.starknet.lib.stark_eip191 import StarkEIP191
from contracts.starknet.lib.eip712 import EIP712
from contracts.starknet.lib.eth_tx import EthTx
from contracts.starknet.lib.array_utils import ArrayUtils
from contracts.starknet.lib.uint256_utils import Uint256Utils

@storage_var
func SessionKey_owner_store(session_public_key: felt) -> (eth_address: felt) {
Expand Down Expand Up @@ -43,6 +44,11 @@ namespace SessionKey {
session_duration: felt,
) {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

EIP712.verify_session_key_init_sig(
r, s, v, salt, eth_address, session_public_key, session_duration
);
Expand Down Expand Up @@ -92,6 +98,11 @@ namespace SessionKey {
ecdsa_ptr: SignatureBuiltin*,
}(r: Uint256, s: Uint256, v: felt, salt: Uint256, session_public_key: felt) {
alloc_locals;

Uint256Utils.assert_valid_uint256(r);
Uint256Utils.assert_valid_uint256(s);
Uint256Utils.assert_valid_uint256(salt);

let (eth_address) = SessionKey_owner_store.read(session_public_key);
with_attr error_message("SessionKey: Session does not exist") {
assert_not_zero(eth_address);
Expand Down
2 changes: 1 addition & 1 deletion contracts/starknet/lib/stark_eip191.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from starkware.starknet.common.syscalls import get_caller_address
from starkware.cairo.common.cairo_builtins import HashBuiltin, SignatureBuiltin
from starkware.cairo.common.math import assert_not_zero
from starkware.cairo.common.uint256 import uint256_eq, Uint256
from starkware.cairo.common.uint256 import uint256_eq
from starkware.cairo.common.alloc import alloc
from starkware.cairo.common.signature import verify_ecdsa_signature
from starkware.starknet.common.syscalls import get_contract_address
Expand Down
14 changes: 14 additions & 0 deletions contracts/starknet/lib/uint256_utils.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// SPDX-License-Identifier: MIT

from starkware.cairo.common.uint256 import Uint256, uint256_check

namespace Uint256Utils {
// Wrapper function to check if `uint256` is a valid `Uint256`.
// Wrapper is needed to have a proper error message.
func assert_valid_uint256{range_check_ptr}(uint256: Uint256) {
with_attr error_message("Uint256Utils: Invalid Uint256") {
uint256_check(uint256);
}
return ();
}
}
8 changes: 7 additions & 1 deletion contracts/starknet/lib/voting.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ from contracts.starknet.lib.proposal_outcome import ProposalOutcome
// Libraries
from contracts.starknet.lib.array_utils import ArrayUtils, Immutable2DArray
from contracts.starknet.lib.felt_utils import FeltUtils
from contracts.starknet.lib.uint256_utils import Uint256Utils

//
// Storage
Expand Down Expand Up @@ -202,8 +203,9 @@ namespace Voting {
assert_not_zero(voting_strategies_len);
assert_not_zero(authenticators_len);
assert_not_zero(executors_len);
Uint256Utils.assert_valid_uint256(proposal_threshold);
Uint256Utils.assert_valid_uint256(quorum);
}
// TODO: maybe use uint256_signed_nn to check proposal_threshold?

// Initialize the storage variables
Voting_voting_delay_store.write(voting_delay);
Expand Down Expand Up @@ -250,6 +252,8 @@ namespace Voting {
) {
Ownable.assert_only_owner();

Uint256Utils.assert_valid_uint256(new_quorum);

let (previous_quorum) = Voting_quorum_store.read();

Voting_quorum_store.write(new_quorum);
Expand Down Expand Up @@ -319,6 +323,8 @@ namespace Voting {
}(new_proposal_threshold: Uint256) {
Ownable.assert_only_owner();

Uint256Utils.assert_valid_uint256(new_proposal_threshold);

let (previous_proposal_threshold) = Voting_proposal_threshold_store.read();

Voting_proposal_threshold_store.write(new_proposal_threshold);
Expand Down

0 comments on commit c71f8f3

Please sign in to comment.