Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update felt252 #44

Merged
merged 4 commits into from
Sep 18, 2023
Merged

Update felt252 #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0c1cd8
update name/difficulty for felt252 detector
technovision99 Sep 5, 2023
887400b
update readme
technovision99 Sep 5, 2023
85e0515
update insta snapshot
technovision99 Sep 16, 2023
23ac8d1
fixed clippy errors
technovision99 Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Num | Detector | What it Detects | Impact | Confidence | Cairo
--- | --- | --- | --- | --- | ---
1 | `controlled-library-call` | Library calls with a user controlled class hash | High | Medium | 1 & 2
2 | `unchecked-l1-handler-from` | Detect L1 handlers without from address check | High | Medium | 1 & 2
3 | `felt252-overflow` | Detect user controlled operations with felt252 type, which is not overflow safe | High | Medium | 1 & 2
3 | `felt252-unsafe-arithmetic` | Detect user controlled operations with felt252 type, which is not overflow/underflow safe | Medium | Medium | 1 & 2
4 | `reentrancy` | Detect when a storage variable is read before an external call and written after | Medium | Medium | 1 & 2
5 | `read-only-reentrancy` | Detect when a view function read a storage variable written after an external call | Medium | Medium | 1 & 2
6 | `unused-events` | Events defined but not emitted | Medium | Medium | 1 & 2
Expand Down
19 changes: 8 additions & 11 deletions src/compilation/utils/felt252_serde.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,14 @@ impl Felt252Serde for StatementIdx {

/// A set of all the supported long generic ids.
static SERDE_SUPPORTED_LONG_IDS: Lazy<OrderedHashSet<&'static str>> = Lazy::new(|| {
OrderedHashSet::from_iter(
[
StorageAddressFromBaseAndOffsetLibfunc::STR_ID,
ContractAddressTryFromFelt252Libfunc::STR_ID,
StorageBaseAddressFromFelt252Libfunc::STR_ID,
StorageAddressTryFromFelt252Trait::STR_ID,
Secp256GetPointFromXLibfunc::<Secp256k1>::STR_ID,
Secp256GetPointFromXLibfunc::<Secp256r1>::STR_ID,
]
.into_iter(),
)
OrderedHashSet::from_iter([
StorageAddressFromBaseAndOffsetLibfunc::STR_ID,
ContractAddressTryFromFelt252Libfunc::STR_ID,
StorageBaseAddressFromFelt252Libfunc::STR_ID,
StorageAddressTryFromFelt252Trait::STR_ID,
Secp256GetPointFromXLibfunc::<Secp256k1>::STR_ID,
Secp256GetPointFromXLibfunc::<Secp256r1>::STR_ID,
])
});
/// A mapping of all the long names when fixing them from the hashed keccak representation.
static LONG_NAME_FIX: Lazy<UnorderedHashMap<BigUint, &'static str>> = Lazy::new(|| {
Expand Down
10 changes: 5 additions & 5 deletions src/detectors/felt252_overflow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ pub struct Felt252Overflow {}

impl Detector for Felt252Overflow {
fn name(&self) -> &str {
"felt252-overflow"
"felt252-unsafe-arithmetic"
}

fn description(&self) -> &str {
"Detect felt252 arithmetic overflow with user-controlled params"
"Detect felt252 arithmetic overflow/underflow with user-controlled params"
}

fn confidence(&self) -> Confidence {
Confidence::Medium
}

fn impact(&self) -> Impact {
Impact::High
Impact::Medium
}

fn run(&self, core: &CoreUnit) -> Vec<Result> {
Expand Down Expand Up @@ -116,7 +116,7 @@ impl Felt252Overflow {
// Not tainted by any parameter, but still uses felt252 type
if tainted_by.is_empty() {
let msg = format!(
"The function {} uses the felt252 operation {}, which is not overflow safe",
"The function {} uses the felt252 operation {}, which is not overflow/underflow safe",
&name, libfunc
);
results.push(Result {
Expand All @@ -127,7 +127,7 @@ impl Felt252Overflow {
});
} else {
let msg = format!(
"The function {} uses the felt252 operation {} with the user-controlled parameters: {}, which is not overflow safe",
"The function {} uses the felt252 operation {} with the user-controlled parameters: {}, which is not overflow/underflow safe",
&name,
libfunc,
taints
Expand Down
12 changes: 6 additions & 6 deletions tests/snapshots/integration_tests__detectors@dead_code.cairo.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ input_file: tests/detectors/dead_code.cairo
---
[
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function dead_code::dead_code::DeadCode::add_1 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow safe",
message: "The function dead_code::dead_code::DeadCode::add_1 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function dead_code::dead_code::DeadCode::add_2 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow safe",
message: "The function dead_code::dead_code::DeadCode::add_2 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow/underflow safe",
},
Result {
impact: Low,
Expand Down
60 changes: 30 additions & 30 deletions tests/snapshots/integration_tests__detectors@felt252_overflow.cairo.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,64 @@ input_file: tests/detectors/felt252_overflow.cairo
---
[
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add uses the felt252 operation felt252_add([0], [1]) -> ([6]) with the user-controlled parameters: [0],[1], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add uses the felt252 operation felt252_add([0], [1]) -> ([6]) with the user-controlled parameters: [0],[1], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_controlled uses the felt252 operation felt252_add([22], [3]) -> ([23]) with the user-controlled parameters: [3], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_controlled uses the felt252 operation felt252_add([22], [3]) -> ([23]) with the user-controlled parameters: [3], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_controlled uses the felt252 operation felt252_add([23], [4]) -> ([24]) with the user-controlled parameters: [23],[4], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_controlled uses the felt252 operation felt252_add([23], [4]) -> ([24]) with the user-controlled parameters: [23],[4], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_uncontrolled uses the felt252 operation felt252_sub([1], [2]) -> ([3]), which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_add_uncontrolled uses the felt252 operation felt252_sub([1], [2]) -> ([3]), which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([6], [5]) -> ([10]) with the user-controlled parameters: [6],[5], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([6], [5]) -> ([10]) with the user-controlled parameters: [6],[5], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([7], [4]) -> ([6]) with the user-controlled parameters: [7],[4], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([7], [4]) -> ([6]) with the user-controlled parameters: [7],[4], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([9], [3]) -> ([8]) with the user-controlled parameters: [9],[3], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_mul_controlled uses the felt252 operation felt252_mul([9], [3]) -> ([8]) with the user-controlled parameters: [9],[3], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_sub_controlled uses the felt252 operation felt252_sub([16], [3]) -> ([17]) with the user-controlled parameters: [3], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_sub_controlled uses the felt252 operation felt252_sub([16], [3]) -> ([17]) with the user-controlled parameters: [3], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_sub_uncontrolled uses the felt252 operation felt252_sub([3], [4]) -> ([5]), which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::bad_sub_uncontrolled uses the felt252 operation felt252_sub([3], [4]) -> ([5]), which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::test_sub_assert uses the felt252 operation felt252_sub([1], [12]) -> ([13]) with the user-controlled parameters: [1], which is not overflow safe",
message: "The function felt252_overflow::felt252_overflow::Felt252Overflow::test_sub_assert uses the felt252 operation felt252_sub([1], [12]) -> ([13]) with the user-controlled parameters: [1], which is not overflow/underflow safe",
},
Result {
impact: Medium,
Expand Down
28 changes: 14 additions & 14 deletions tests/snapshots/integration_tests__detectors@unchecked_l1_handler_from.cairo.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@ input_file: tests/detectors/unchecked_l1_handler_from.cairo
[
Result {
impact: High,
name: "felt252-overflow",
name: "unchecked-l1-handler-from",
confidence: Medium,
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::bad uses the felt252 operation felt252_add([1], [2]) -> ([3]) with the user-controlled parameters: [1], which is not overflow safe",
message: "The L1 handler function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::bad does not check the L1 from address",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::check_recursive uses the felt252 operation felt252_add([3], [50]) -> ([51]), which is not overflow safe",
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::bad uses the felt252 operation felt252_add([1], [2]) -> ([3]) with the user-controlled parameters: [1], which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::check_recursive uses the felt252 operation felt252_sub([11], [9]) -> ([10]), which is not overflow safe",
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::check_recursive uses the felt252 operation felt252_add([3], [50]) -> ([51]), which is not overflow/underflow safe",
},
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::good3 uses the felt252 operation felt252_add([14], [15]) -> ([16]) with the user-controlled parameters: [14], which is not overflow safe",
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::check_recursive uses the felt252 operation felt252_sub([11], [9]) -> ([10]), which is not overflow/underflow safe",
},
Result {
impact: High,
name: "unchecked-l1-handler-from",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The L1 handler function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::bad does not check the L1 from address",
message: "The function unchecked_l1_handler_from::unchecked_l1_handler_from::UncheckedL1HandlerFrom::good3 uses the felt252 operation felt252_add([14], [15]) -> ([16]) with the user-controlled parameters: [14], which is not overflow/underflow safe",
},
]
6 changes: 3 additions & 3 deletions tests/snapshots/integration_tests__detectors@unused_return.cairo.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ input_file: tests/detectors/unused_return.cairo
---
[
Result {
impact: High,
name: "felt252-overflow",
impact: Medium,
name: "felt252-unsafe-arithmetic",
confidence: Medium,
message: "The function unused_return::unused_return::UnusedReturn::f_5 uses the felt252 operation felt252_mul([14], [15]) -> ([16]), which is not overflow safe",
message: "The function unused_return::unused_return::UnusedReturn::f_5 uses the felt252 operation felt252_mul([14], [15]) -> ([16]), which is not overflow/underflow safe",
},
Result {
impact: Medium,
Expand Down