|
1 | 1 | # Incorrect Felt Comparison |
2 | 2 |
|
3 | | -In cairo, there are two methods for comparison, in particular for the less than or equal to operator we have the methods `assert_le` and `assert_nn_le`. `assert_le` asserts that a number a is less than or equal to b, regardless of the size of a, while `assert_nn_le` will also assert that a is non-negative, ie not greater than the `RANGE_CHECK_BOUND` value of `2^128`: https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/math.cairo#L66-L67 |
| 3 | +In Cairo, there are two builtin methods for the less than or equal to comparison operator: `assert_le` and `assert_nn_le`. `assert_le` asserts that a number `a` is less than or equal to `b`, regardless of the size of `a`, while `assert_nn_le` additionally asserts that `a` is non-negative, i.e. not greater than or equal to the `RANGE_CHECK_BOUND` value of `2^128`: https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/math.cairo#L66-L67 |
4 | 4 |
|
5 | 5 | # Example |
6 | 6 |
|
7 | | -Suppose that a codebase uses the following checks regarding a hypothetical ERC20 token. In the first function, it may be possible that `value` is in fact greater than `max_supply`, yet because the function does not verify `value <0` the assertion will incorrectly pass. The second function, however, asserts that `0 < value < max_supply`, which will correctly not let an incorrect `value` go through the assertion. |
| 7 | +Suppose that a codebase uses the following checks regarding a hypothetical ERC20 token. In the first function, it may be possible that `value` is in fact greater than `max_supply`, yet because the function does not verify `value >= 0` the assertion will incorrectly pass. The second function, however, asserts that `0 <= value <= max_supply`, which will correctly not let an incorrect `value` go through the assertion. |
8 | 8 |
|
9 | 9 | ```cairo |
10 | 10 | @storage_var |
11 | | -func max_supply() -> (res: felt): |
12 | | -end |
| 11 | +func max_supply() -> (res: felt) { |
| 12 | +} |
13 | 13 |
|
14 | 14 | @external |
15 | | -func bad_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(): |
16 | | - let (value: felt) = ERC20.total_supply() |
17 | | - assert_le{range_check_ptr=range_check_ptr}(value, max_supply.read()) |
| 15 | +func bad_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}() { |
| 16 | + let (value: felt) = ERC20.total_supply(); |
| 17 | + assert_le{range_check_ptr=range_check_ptr}(value, max_supply.read()); |
18 | 18 |
|
19 | | - # do something... |
| 19 | + // do something... |
20 | 20 |
|
21 | | - return () |
22 | | -end |
| 21 | + return (); |
| 22 | +} |
23 | 23 |
|
24 | 24 | @external |
25 | | -func better_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(): |
26 | | - let (value: felt) = ERC20.total_supply() |
27 | | - assert_nn_le{range_check_ptr=range_check_ptr}(value, max_supply.read()) |
| 25 | +func better_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}() { |
| 26 | + let (value: felt) = ERC20.total_supply(); |
| 27 | + assert_nn_le{range_check_ptr=range_check_ptr}(value, max_supply.read()); |
28 | 28 |
|
29 | | - # do something... |
| 29 | + // do something... |
30 | 30 |
|
31 | | - return () |
| 31 | + return (); |
32 | 32 |
|
33 | 33 | |
34 | | -end |
| 34 | +} |
35 | 35 | ``` |
36 | 36 |
|
37 | 37 |
|
38 | 38 |
|
39 | 39 | # Mitigations |
40 | | -Review all felt comparisons closely. Determine what sort of behavior the comparison should have, and if `assert_nn_le` is more appropriate. |
| 40 | +Review all felt comparisons closely. Determine what sort of behavior the comparison should have, and if `assert_nn_le` is more appropriate. |
0 commit comments