Merge pull request #338 from crytic/update-cairo

Update cairo

Author: montyly

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

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

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

@@ -1,14 +1,27 @@
-# Arithmetic Overflow
+# Arithmetic Overflow with Felt Type
 
-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.
+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.
 
-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.
+## Example
 
-## Attack Scenarios
+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.
 
-## Mitigations
+```Cairo
+
+struct Storage {
+    balances: LegacyMap<ContractAddress, felt252>
+}
+
+fn check_balance(self: @ContractState, amt: felt252) {
+    let caller = get_caller_address();
+    let balance = self.balances.read(caller);
+    assert(balance - amt >= 0);
+}
 
-- Always add checks for overflow when working with felts or Uint256s directly.
-- 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.
+```
+
+## Mitigations
 
-## Examples
+- Always add checks for overflow when working with felts directly.
+- Use the default signed integer types unless a felt is explicitly required.
+- Consider using Caracal, as it comes with a detector for checking potential overflows when doing felt252 arithmetic operaitons.

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

@@ -1,4 +1,4 @@
-# Consider L1 to L2 Message Failure
+# L1 to L2 Message Failure
 
 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.