Skip to content

Commit 03d3129

Browse files
updated cairo section for cairo 2
1 parent eb6ea12 commit 03d3129

File tree

12 files changed

+137
-281
lines changed

12 files changed

+137
-281
lines changed

not-so-smart-contracts/cairo/README.md

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,12 @@ Each _Not So Smart Contract_ consists of a standard set of information:
1616

1717
| Not So Smart Contract | Description |
1818
| ------------------------------------------------------------------------------ | ------------------------------------------------------------ |
19-
| [Improper access controls](access_controls) | Flawed access controls due to StarkNet account abstraction |
20-
| [Integer division errors](integer_division) | Unforeseen results from division in a finite field |
21-
| [View state modifications](view_state) | Lack of prevention for state modifications in view functions |
22-
| [Arithmetic overflow](arithmetic_overflow) | Insecure arithmetic in Cairo by default |
19+
| [Arithmetic overflow](arithmetic_overflow) | Insecure arithmetic in Cairo for the felt252 type |
20+
| [L1 to L2 Address Conversion](l1_to_l2_address_conversion) | Essential L2 address checks for L1 to L2 messaging |
21+
| [L1 to L2 message failure](l1_to_l2_message_failure) | Messages sent from L1 may not be processed by the sequencer |
22+
| [Overconstrained L1 <-> L2 interaction](overconstrained_l1_l2_interaction) | Asymmetrical checks on the L1 or L2 side can cause a DOS |
2323
| [Signature replays](replay_protection) | Necessary robust reuse protection due to account abstraction |
24-
| [L1 to L2 Address Conversion](L1_to_L2_address_conversion) | Essential L2 address checks for L1 to L2 messaging |
25-
| [Incorrect Felt Comparison](incorrect_felt_comparison) | Unexpected results from felt comparison |
26-
| [Namespace Storage Var Collision](namespace_storage_var_collision) | Storage variables unscoped by namespaces |
27-
| [Dangerous Public Imports in Libraries](dangerous_public_imports_in_libraries) | Ability to call nonimported external functions |
24+
| [Unchecked from address in L1 Handler](unchecked_from_address_in_l1_handler) | Access control issue when sending messages from L1 to L2 |
2825

2926
## Credits
3027

not-so-smart-contracts/cairo/access_controls/README.md

Lines changed: 0 additions & 42 deletions
This file was deleted.

not-so-smart-contracts/cairo/arithmetic_overflow/README.md

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,29 @@
1-
# Arithmetic Overflow
1+
# Arithmetic Overflow with Felt Type
22

3-
The default primitive type, the field element (felt), behaves much like an integer in other languages, but there are a few important differences to keep in mind. A felt can be interpreted as a signed integer in the range (-P/2, P/2) or as an unsigned integer in the range (0, P]. P represents the prime used by Cairo, which is currently a 252-bit number. Arithmetic operations using felts are unchecked for overflow, which can lead to unexpected results if not properly accounted for. Since the range of values includes both negative and positive values, multiplying two positive numbers can result in a negative value and vice versa—multiplying two negative numbers does not always produce a positive result.
3+
The default primitive type, the field element (felt), behaves much like an integer in other languages, but there are a few important differences to keep in mind. A felt can be interpreted as an unsigned integer in the range [0, P], where P, a 252 bit prime, represents the order of the field used by Cairo. Arithmetic operations using felts are unchecked for overflow or underflow, which can lead to unexpected results if not properly accounted for. Do note that Cairo's builtin primitives for unsigned integers are overflow/underflow safe and will revert.
44

5-
StarkNet also provides the Uint256 module, offering developers a more typical 256-bit integer. However, the arithmetic provided by this module is also unchecked, so overflow is still a concern. For more robust integer support, consider using SafeUint256 from OpenZeppelin's Contracts for Cairo.
65

7-
## Attack Scenarios
6+
## Example
87

8+
The following simplified code highlights the risk of felt underflow. The `check_balance` function is used to validate if a user has a large enough balance. However, the check is faulty because passing an amount such that `amt > balance` will underflow and the check will be true.
9+
10+
```Cairo
11+
12+
struct Storage {
13+
balances: LegacyMap<ContractAddress, felt252>
14+
}
15+
16+
fn check_balance(self: @ContractState, amt: felt252) {
17+
let caller = get_caller_address();
18+
let balance = self.balances.read(caller);
19+
assert(balance - amt >= 0);
20+
}
21+
22+
```
923
## Mitigations
1024

11-
- Always add checks for overflow when working with felts or Uint256s directly.
12-
- Consider using the [OpenZeppelin Contracts for Cairo's SafeUint256 functions](https://github.com/OpenZeppelin/cairo-contracts/blob/main/src/openzeppelin/security/safemath/library.cairo) instead of performing arithmetic directly.
25+
- Always add checks for overflow when working with felts directly.
26+
- Use the default signed integer types unless a felt is explicitly required.
27+
- Consider using Caracal, as it comes with a detector for checking potential overflows when doing felt252 arithmetic operaitons.
28+
1329

14-
## Examples

not-so-smart-contracts/cairo/dangerous_public_imports_in_libraries/README.md

Lines changed: 0 additions & 54 deletions
This file was deleted.

not-so-smart-contracts/cairo/incorrect_felt_comparison/README.md

Lines changed: 0 additions & 47 deletions
This file was deleted.

not-so-smart-contracts/cairo/integer_division/README.md

Lines changed: 0 additions & 39 deletions
This file was deleted.

not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md renamed to not-so-smart-contracts/cairo/l1_to_l2_message_failure/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Consider L1 to L2 Message Failure
1+
# L1 to L2 Message Failure
22

33
In Starknet, Ethereum contracts can send messages from L1 to L2 using a bridge. However, it is not guaranteed that the message will be processed by the sequencer. For instance, a message can fail to be processed if there is a sudden spike in the gas price and the value provided is too low. To address this issue, Starknet developers have provided an API to cancel ongoing messages.
44

not-so-smart-contracts/cairo/namespace_storage_var_collision/README.md

Lines changed: 0 additions & 54 deletions
This file was deleted.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Overconstrained L1 <-> L2 interaction
2+
When interacting with contracts that are designed to interact with both L1 and L2, care must be taken that the checks and validations on both sides are symmetrical. If one side has more validations than the other, this could create a situation where a user performs an action on one side, but is unable to perform the corresponding action on the other side, leading to a loss of funds or a denial of service.
3+
4+
5+
## Example
6+
7+
The following Starknet bridge contract allows for permissionless deposit to any address from L1 via the `deposit_to_L2`. In particular, someone can deposit tokens to the `bad_address`.However the tokens will be trapped on L2 because the L2 contract's `deposit_from_L1` function is not permissionless and prevents `bad_address` from being the recipient.
8+
9+
```solidity
10+
uint256 public immutable DEPOSIT_SELECTOR;
11+
address public immutable MESSENGER_CONTRACT;
12+
address public immutable L2_BRIDGE_ADDRESS;
13+
14+
constructor(uint256 _selector, address _messenger, address _bridge) {
15+
DEPOSIT_SELECTOR = _selector;
16+
MESSENGER_CONTRACT = _messenger;
17+
L2_BRIDGE_ADDRESS = _bridge;
18+
19+
}
20+
21+
function depositToL2(uint256[] calldata payload) external {
22+
require(owner == msg.sender, "not owner");
23+
IStarknetMessaging(MESSENGER_CONTRACT).sendMessageToL2(L2_BRIDGE_ADDRESS, DEPOSIT_SELECTOR, payload);
24+
}
25+
```
26+
27+
```Cairo
28+
#[storage]
29+
struct Storage {
30+
owner: ContractAddress,
31+
l1_bridge: EthAddress,
32+
bad_address: ContractAddress
33+
34+
}
35+
36+
#[l1_handler]
37+
fn deposit_from_l1(ref self:ContractState, from_address: felt252, recipient: ContractAddress, amount) {
38+
assert(from_address == l1_bridge, "not bridge");
39+
assert(recipient != bad_address, "not allowed to deposit");
40+
//deposit logic
41+
[...]
42+
}
43+
44+
```
45+
## Mitigations
46+
47+
- Make sure to validate that the checks on both the L1 and L2 side are similar enough to prevent unexpected behavior. Ensure that any unsymmetric validations on either side cannot lead to a tokens being trapped or any other denial of service.

0 commit comments

Comments
 (0)