-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge branch 'release/0.2.16-nightly-2024-07-11' into 294-turn-detect…
…ions-onoff-feature
- Loading branch information
Showing
82 changed files
with
2,281 additions
and
619 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| [package] | ||
| edition = "2021" | ||
| name = "dynamic-instance-storage" | ||
| version = "0.1.0" | ||
|
|
||
| [lib] | ||
| crate-type = ["cdylib"] | ||
|
|
||
| [dependencies] | ||
| clippy_utils = { workspace = true } | ||
| dylint_linting = { workspace = true } | ||
| if_chain = { workspace = true } | ||
| utils = { workspace = true } | ||
|
|
||
| [package.metadata.rust-analyzer] | ||
| rustc_private = true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| #![feature(rustc_private)] | ||
|
|
||
| extern crate rustc_hir; | ||
| extern crate rustc_middle; | ||
| extern crate rustc_span; | ||
|
|
||
| use clippy_utils::diagnostics::span_lint; | ||
| use if_chain::if_chain; | ||
| use rustc_hir::{ | ||
| intravisit::{walk_expr, FnKind, Visitor}, | ||
| Body, Expr, ExprKind, FnDecl, | ||
| }; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_middle::ty::{Ty, TyKind}; | ||
| use rustc_span::{def_id::LocalDefId, Span, Symbol}; | ||
| use utils::{get_node_type_opt, is_soroban_storage, SorobanStorageType}; | ||
|
|
||
| const LINT_MESSAGE: &str = "This function may lead to excessive instance storage growth, which could increase execution costs or potentially cause DoS"; | ||
|
|
||
| dylint_linting::impl_late_lint! { | ||
| pub DYNAMIC_INSTANCE_STORAGE, | ||
| Warn, | ||
| LINT_MESSAGE, | ||
| DynamicInstanceStorage, | ||
| { | ||
| name: "Dynamic Instance Storage Analyzer", | ||
| long_message: "Detects potential misuse of instance storage that could lead to unnecessary growth or storage-related vulnerabilities.", | ||
| severity: "Warning", | ||
| help: "https://coinfabrik.github.io/scout-soroban/docs/detectors/dynamic-instance-storage", | ||
| vulnerability_class: "Resource Management", | ||
| } | ||
| } | ||
|
|
||
| #[derive(Default)] | ||
| struct DynamicInstanceStorage; | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for DynamicInstanceStorage { | ||
| fn check_fn( | ||
| &mut self, | ||
| cx: &LateContext<'tcx>, | ||
| _: FnKind<'tcx>, | ||
| _: &'tcx FnDecl<'tcx>, | ||
| body: &'tcx Body<'tcx>, | ||
| span: Span, | ||
| _: LocalDefId, | ||
| ) { | ||
| if span.from_expansion() { | ||
| return; | ||
| } | ||
|
|
||
| let mut storage_warn_visitor = DynamicInstanceStorageVisitor { cx }; | ||
| storage_warn_visitor.visit_body(body); | ||
| } | ||
| } | ||
|
|
||
| struct DynamicInstanceStorageVisitor<'a, 'tcx> { | ||
| cx: &'a LateContext<'tcx>, | ||
| } | ||
|
|
||
| impl<'a, 'tcx> Visitor<'tcx> for DynamicInstanceStorageVisitor<'a, 'tcx> { | ||
| fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { | ||
| if_chain! { | ||
| // Detect calls to `set` method | ||
| if let ExprKind::MethodCall(path, receiver, args, _) = &expr.kind; | ||
| if path.ident.name == Symbol::intern("set"); | ||
| // Get the type of the receiver and check if it is an instance storage | ||
| if let Some(receiver_ty) = get_node_type_opt(self.cx, &receiver.hir_id); | ||
| if is_soroban_storage(self.cx, receiver_ty, SorobanStorageType::Instance); | ||
| // Check if the value being set is a dynamic type | ||
| if args.len() >= 2; | ||
| if let Some(value_type) = get_node_type_opt(self.cx, &args[1].hir_id); | ||
| if is_dynamic_type(self.cx, &value_type); | ||
| then { | ||
| span_lint(self.cx, DYNAMIC_INSTANCE_STORAGE, expr.span, LINT_MESSAGE) | ||
| } | ||
| } | ||
|
|
||
| walk_expr(self, expr) | ||
| } | ||
| } | ||
|
|
||
| fn is_dynamic_type(cx: &LateContext, ty: &Ty) -> bool { | ||
| match ty.kind() { | ||
| TyKind::Str => true, | ||
| TyKind::Slice(_) => true, | ||
| TyKind::Dynamic(..) => true, | ||
| TyKind::Array(element_ty, _) => is_dynamic_type(cx, element_ty), | ||
| TyKind::Adt(adt_def, _) => { | ||
| let type_name = cx.tcx.item_name(adt_def.did()); | ||
| matches!(type_name.as_str(), "Vec" | "String" | "Map" | "LinkedList") | ||
| } | ||
| TyKind::RawPtr(ty, _) => is_dynamic_type(cx, ty), | ||
| TyKind::Ref(_, ty, _) => is_dynamic_type(cx, ty), | ||
| TyKind::Tuple(substs) => substs.iter().any(|ty| is_dynamic_type(cx, &ty)), | ||
| _ => false, | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,55 @@ | ||
| # Divide before multiply | ||
|
|
||
| ### What it does | ||
| ## Description | ||
|
|
||
| Checks the existence of a division before a multiplication. | ||
| - Category: `Arithmetic` | ||
| - Severity: `Medium` | ||
| - Detectors: [`divide-before-multiply`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/divide-before-multiply) | ||
| - Test Cases: [`divide-before-multiply-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/divide-before-multiply/divide-before-multiply-1) [`divide-before-multiply-2`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/divide-before-multiply/divide-before-multiply-2) [`divide-before-multiply-3`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/divide-before-multiply/divide-before-multiply-3) | ||
|
|
||
| In Rust, the order of operations can influence the precision of the result, especially in integer arithmetic. | ||
|
|
||
| ### Why is this bad? | ||
| ## Why is this bad? | ||
|
|
||
| Performing a division operation before multiplication can lead to a loss of precision. It might even result in an unintended zero value. | ||
| Performing a division operation before a multiplication can lead to a loss of precision as division between integers might return zero. | ||
|
|
||
| ### Example | ||
| ## Issue example | ||
|
|
||
| Consider the following `Soroban` contract: | ||
|
|
||
| ```rust | ||
| // Example 1 - Vulnerable | ||
| let x = 10; | ||
| let y = 6; | ||
| let z = x / y * 3; // z evaluates to 3 | ||
|
|
||
| // Example 2 - Vulnerable | ||
| let a = 1; | ||
| let b = 2; | ||
| let c = a / b * 3; // c evaluates to 0 | ||
|
|
||
| pub fn split_profit(percentage: u64, total_profit: u64) -> u64 { | ||
| (percentage / 100) * total_profit | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| Use instead: | ||
| In this contract, the `split_profit` function divides the `percentage` by `100` before multiplying it with `total_profit`. This could lead to a loss of precision if `percentage` is less than `100` as the division would return `0`. This could lead to incorrect calculations and potential financial loss in a real-world smart contract. | ||
|
|
||
|
|
||
| The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/divide-before-multiply/divide-before-multiply-1/vulnerable-example). | ||
|
|
||
|
|
||
| ## Remediated example | ||
|
|
||
| Reverse the order of operations to ensure multiplication occurs before division. | ||
|
|
||
| ```rust | ||
| // Example 1 - Remediated | ||
| let x = 10; | ||
| let y = 6; | ||
| let z = x * 3 / y; // z evaluates to 5 | ||
|
|
||
| // Example 2 - Remediated | ||
| let a = 1; | ||
| let b = 2; | ||
| let c = a * 3 / b; // c evaluates to 1 | ||
|
|
||
| pub fn split_profit(&self, percentage: u64, total_profit: u64) -> u64 { | ||
| (percentage * total_profit) / 100 | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| ### Implementation | ||
| The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/divide-before-multiply/divide-before-multiply-1/remediated-example). | ||
|
|
||
| ## How is it detected? | ||
|
|
||
| The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/divide-before-multiply). | ||
| Checks the existence of a division before a multiplication. | ||
|
|
||
| ## References | ||
|
|
||
| [Rust documentation: `Integer Division`](https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators) | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,47 +1,74 @@ | ||
| # Avoid unsafe block | ||
|
|
||
| ### What it does | ||
| ## Description | ||
|
|
||
| Checks for usage of `unsafe` blocks. | ||
| - Category: `Validations and error handling` | ||
| - Severity: `Critical` | ||
| - Detector: [`avoid-unsafe-block`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block) | ||
| - Test Cases: [`avoid-unsafe-block-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1) | ||
|
|
||
| The use of unsafe blocks in Rust is generally discouraged due to the potential risks it poses to the safety and reliability of the code. Rust's primary appeal lies in its ability to provide memory safety guarantees, which are largely enforced through its ownership and type systems. When you enter an unsafe block, you're effectively bypassing these safety checks. These blocks require the programmer to manually ensure that memory is correctly managed and accessed, which is prone to human error and can be challenging even for experienced developers. Therefore, unsafe blocks should only be used when absolutely necessary and when the safety of the operations within can be assured. | ||
|
|
||
| ### Why is this bad? | ||
| ## Why is this bad? | ||
|
|
||
| `unsafe` blocks should not be used unless absolutely necessary. The use of unsafe blocks in Rust is discouraged because they bypass Rust's memory safety checks, potentially leading to issues like undefined behavior and security vulnerabilities. | ||
|
|
||
| ### Example | ||
| ## Issue example | ||
|
|
||
| ```rust | ||
| pub fn unsafe_function(n: u64) -> u64 { | ||
| unsafe { | ||
| let mut i = n as f64; | ||
| let mut y = i.to_bits(); | ||
| y = 0x5fe6ec85e7de30da - (y >> 1); | ||
| i = f64::from_bits(y); | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
| Consider the following `Soroban` contract: | ||
|
|
||
| let result_ptr: *mut f64 = &mut i; | ||
| let result = *result_ptr; | ||
| ```rust | ||
| #[contractimpl] | ||
| impl AvoidUnsafeBlock { | ||
| pub fn unsafe_function(n: u64) -> u64 { | ||
| unsafe { | ||
| let mut i = n as f64; | ||
| let mut y = i.to_bits(); | ||
| y = 0x5fe6ec85e7de30da - (y >> 1); | ||
| i = f64::from_bits(y); | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
|
|
||
| result.to_bits() | ||
| } | ||
| let result_ptr: *mut f64 = &mut i; | ||
|
|
||
| (*result_ptr).to_bits() | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Use instead: | ||
| In this example we can see that it creates a raw pointer named `result_ptr`. Then `(*result_ptr).to_bits()` dereferences the raw pointer. This directly accesses the memory location and calls the `to_bits` method on the value stored at that location. | ||
|
|
||
| Raw pointers bypass Rust's type safety system and memory management features. If something goes wrong with the calculations or the value of n, dereferencing the pointer could lead to a memory access violations or undefined behavior. | ||
|
|
||
| ```rust | ||
| pub fn unsafe_function(n: u64) -> u64 { | ||
| The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1/vulnerable-example). | ||
|
|
||
|
|
||
| ## Remediated example | ||
|
|
||
| By removing the raw pointer, the following version eliminates the issue associated with dereferencing memory in an unsafe way. Rust's type safety checks ensure memory is accessed correctly, preventing the potential issues mentioned earlier. | ||
|
|
||
| ```rust | ||
| #[contractimpl] | ||
| impl AvoidUnsafeBlock { | ||
| pub fn unsafe_function(n: u64) -> u64 { | ||
| let mut i = n as f64; | ||
| let mut y = i.to_bits(); | ||
| y = 0x5fe6ec85e7de30da - (y >> 1); | ||
| i = f64::from_bits(y); | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
| i *= 1.5 - 0.5 * n as f64 * i * i; | ||
| result.to_bits() | ||
| } | ||
| i.to_bits() | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Implementation | ||
| The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1/remediated-example). | ||
|
|
||
| ## How is it detected? | ||
|
|
||
| Checks for usage of `unsafe` blocks. | ||
|
|
||
|
|
||
|
|
||
| The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block). | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,56 @@ | ||
| # Soroban version | ||
|
|
||
| ### What it does | ||
| ## Description | ||
|
|
||
| Warns you if you are using an old version of Soroban in the `Cargo.toml`. | ||
| - Category: `Best practices` | ||
| - Severity: `Enhacement` | ||
| - Detector: [`soroban-version`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version) | ||
| - Test Cases: [`soroban-version-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1) | ||
|
|
||
| Using an outdated version of Soroban can lead to issues in our contract. It's a good practice to use the latest version. | ||
|
|
||
| ### Why is this bad? | ||
| ## Why is this bad? | ||
|
|
||
| Using an old version of Soroban can be dangerous, as it may have bugs or security issues. | ||
|
|
||
| ### Example | ||
| ## Issue example | ||
|
|
||
|
|
||
| Consider the following `Cargo.toml`: | ||
|
|
||
| ```toml | ||
| [dependencies] | ||
| soroban-sdk = { version = "=20.0.0" } | ||
| [dependencies] | ||
| soroban-sdk = { version = "=19.0.0" } | ||
|
|
||
| [dev-dependencies] | ||
| soroban-sdk = { version = "=20.0.0", features = ["testutils"] } | ||
| [dev_dependencies] | ||
| soroban-sdk = { version = "=19.0.0", features = ["testutils"] } | ||
| ``` | ||
|
|
||
| Instead, use the latest available version in the `Cargo.toml`. | ||
| Problems can arise if the version is not updated to the latest available. | ||
|
|
||
| The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1/vulnerable-example). | ||
|
|
||
|
|
||
| ## Remediated example | ||
|
|
||
| ```toml | ||
| [dependencies] | ||
| // Use the latest version available. | ||
| soroban-sdk = { workspace = true } | ||
|
|
||
| [dev_dependencies] | ||
| soroban-sdk = { workspace = true, features = ["testutils"] } | ||
| ``` | ||
|
|
||
| The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1/remediated-example) | ||
|
|
||
| ## How is it detected? | ||
|
|
||
| Warns you if you are using an old version of Soroban in the `Cargo.toml`. | ||
|
|
||
| ## References | ||
|
|
||
| - [Floating Pragma](https://swcregistry.io/docs/SWC-103/) | ||
| - [outdated Compiler Version](https://swcregistry.io/docs/SWC-102/) | ||
|
|
||
| ### Implementation | ||
|
|
||
| The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version). |
Oops, something went wrong.