From 4163e9393ef77e484891c4451e2e50bc10b2580c Mon Sep 17 00:00:00 2001 From: user Date: Mon, 6 May 2024 09:56:02 -0300 Subject: [PATCH 01/21] Add incorrect exponentiation detector --- detectors/incorrect-exponentiation/Cargo.toml | 16 ++++ detectors/incorrect-exponentiation/src/lib.rs | 81 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 detectors/incorrect-exponentiation/Cargo.toml create mode 100644 detectors/incorrect-exponentiation/src/lib.rs diff --git a/detectors/incorrect-exponentiation/Cargo.toml b/detectors/incorrect-exponentiation/Cargo.toml new file mode 100644 index 00000000..012d8d90 --- /dev/null +++ b/detectors/incorrect-exponentiation/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "incorrect-exponentiation" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +scout-audit-clippy-utils = { workspace = true } +dylint_linting = { workspace = true } +if_chain = { workspace = true } + + +[package.metadata.rust-analyzer] +rustc_private = true diff --git a/detectors/incorrect-exponentiation/src/lib.rs b/detectors/incorrect-exponentiation/src/lib.rs new file mode 100644 index 00000000..dbd2c5ae --- /dev/null +++ b/detectors/incorrect-exponentiation/src/lib.rs @@ -0,0 +1,81 @@ +#![feature(rustc_private)] +#![warn(unused_extern_crates)] + +extern crate rustc_hir; +extern crate rustc_span; + +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::Visitor; +use rustc_hir::intravisit::{walk_expr, FnKind}; +use rustc_hir::{Body, FnDecl}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::Span; +use scout_audit_clippy_utils::diagnostics::span_lint_and_help; + +const LINT_MESSAGE: &str = "'^' It is not an exponential operator. It is a bitwise XOR."; +const LINT_HELP: &str = "If you want to use XOR, use bitxor(). If you want to raise a number use .checked_pow() or .pow() "; + +dylint_linting::declare_late_lint! { + pub INCORRECT_EXPONENTIATION, + Warn, + LINT_MESSAGE, + { + name: "Incorrect Exponentiation", + long_message: LINT_MESSAGE, + severity: "Critical", + help: "https://coinfabrik.github.io/scout/docs/vulnerabilities/incorrect-exponentiation", + vulnerability_class: "Arithmetic", + } + +} + +impl<'tcx> LateLintPass<'tcx> for IncorrectExponentiation { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _: Span, + _: LocalDefId, + ) { + struct IncorrectExponentiationStorage { + span: Vec, + incorrect_exponentiation: bool, + } + + impl<'tcx> Visitor<'tcx> for IncorrectExponentiationStorage { + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if let ExprKind::Binary(op, _, _) = &expr.kind { + if op.node == rustc_hir::BinOpKind::BitXor { + self.incorrect_exponentiation = true; + self.span.push(expr.span); + } + } + + walk_expr(self, expr); + } + } + + let mut expon_storage = IncorrectExponentiationStorage { + span: Vec::new(), + incorrect_exponentiation: false, + }; + + walk_expr(&mut expon_storage, body.value); + + if expon_storage.incorrect_exponentiation { + for span in expon_storage.span.iter() { + span_lint_and_help( + cx, + INCORRECT_EXPONENTIATION, + *span, + LINT_MESSAGE, + None, + LINT_HELP, + ); + } + } + } +} From 9f2024ce4abbf60e52c3a941b06552682530674c Mon Sep 17 00:00:00 2001 From: user Date: Mon, 6 May 2024 10:53:23 -0300 Subject: [PATCH 02/21] Add incorrect exponentiation test cases --- .../incorrect-exponentiation/Cargo.toml | 22 +++++++ .../remediated-example/Cargo.toml | 16 +++++ .../remediated-example/src/lib.rs | 62 +++++++++++++++++++ .../vulnerable-example/Cargo.toml | 16 +++++ .../vulnerable-example/src/lib.rs | 62 +++++++++++++++++++ 5 files changed, 178 insertions(+) create mode 100644 test-cases/incorrect-exponentiation/Cargo.toml create mode 100644 test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml create mode 100644 test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs create mode 100644 test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml create mode 100644 test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs diff --git a/test-cases/incorrect-exponentiation/Cargo.toml b/test-cases/incorrect-exponentiation/Cargo.toml new file mode 100644 index 00000000..3db3ca7b --- /dev/null +++ b/test-cases/incorrect-exponentiation/Cargo.toml @@ -0,0 +1,22 @@ +[workspace] +exclude = [".cargo", "target"] +members = ["incorrect-exponentiation-*/*"] +resolver = "2" + +[workspace.dependencies] +soroban-sdk = "20.3.2" + +[profile.release] +opt-level = "z" +overflow-checks = true +debug = 0 +strip = "symbols" +debug-assertions = false +panic = "abort" +codegen-units = 1 +lto = true + +# For more information about this profile see https://soroban.stellar.org/docs/basic-tutorials/logging#cargotoml-profile +[profile.release-with-logs] +inherits = "release" +debug-assertions = true diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml new file mode 100644 index 00000000..ee9f0b0a --- /dev/null +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "incorrect-exponentiation-remediated-1" +version = "0.0.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs new file mode 100644 index 00000000..c1554c4d --- /dev/null +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -0,0 +1,62 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, contracttype, Env}; + +#[contracttype] +#[derive(Clone)] +enum DataKey { + Data, +} + +#[contract] +pub struct IncorrectExponentiation; + +#[contractimpl] +impl IncorrectExponentiation { + + pub fn init(e: Env){ + e.storage() + .instance() + .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); + } + + pub fn set_data(e: Env, new_data: u128) { + e.storage() + .instance() + .set::(&DataKey::Data, &new_data); + } + + pub fn exp_data_3(e: Env) -> u128 { + let data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + + return data.pow(3); + } + +} + +#[cfg(test)] +mod tests { + use soroban_sdk::{testutils, Address, Env}; + + use crate::{IncorrectExponentiation, IncorrectExponentiationClient}; + + #[test] + fn simple_test() { + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let user =
::generate(&env); + + + client.init(); + client.set_data(&10_u128); + + assert_eq!(client.exp_data_3(), 1000); +} +} + + diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..0300c6ee --- /dev/null +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "incorrect-exponentiation-vulnerable-1" +version = "0.0.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..c2259eff --- /dev/null +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -0,0 +1,62 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, contracttype, Env}; + +#[contracttype] +#[derive(Clone)] +enum DataKey { + Data, +} + +#[contract] +pub struct IncorrectExponentiation; + +#[contractimpl] +impl IncorrectExponentiation { + + pub fn init(e: Env){ + e.storage() + .instance() + .set::(&DataKey::Data, &((255_u128^2) - 1)); + } + + pub fn set_data(e: Env, new_data: u128) { + e.storage() + .instance() + .set::(&DataKey::Data, &new_data); + } + + pub fn exp_data_3(e: Env) -> u128 { + let mut data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + + data = data ^ 3; + return data; + } + +} + +#[cfg(test)] +mod tests { + use soroban_sdk::{testutils, Address, Env}; + + use crate::{IncorrectExponentiation, IncorrectExponentiationClient}; + + #[test] + fn simple_test() { + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let user =
::generate(&env); + + + client.init(); + client.set_data(&10_u128); + + //EXPECTED TO FAIL TO SHOW THAT ^ IS NOT .pow() + assert_eq!(client.exp_data_3(), 1000); +} +} From a086797aadcdef745eb5cc43544e40b5464329a0 Mon Sep 17 00:00:00 2001 From: Nacho Gutman Date: Mon, 6 May 2024 11:31:57 -0300 Subject: [PATCH 03/21] Add incorrect exponentiation vulnerability documentation --- .../21-incorrect-exponentiation.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/docs/vulnerabilities/21-incorrect-exponentiation.md diff --git a/docs/docs/vulnerabilities/21-incorrect-exponentiation.md b/docs/docs/vulnerabilities/21-incorrect-exponentiation.md new file mode 100644 index 00000000..4e9ef330 --- /dev/null +++ b/docs/docs/vulnerabilities/21-incorrect-exponentiation.md @@ -0,0 +1,51 @@ +# Incorrect Exponentiation + +## Description + +- Vulnerability Category: `Arithmetic` +- Vulnerability Severity: `Critical` +- Detectors: [`incorrect-exponentiation`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation) +- Test Cases: [`incorrect-exponentiation-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1) + + +The operator `^` is not an exponential operator, it is a bitwise XOR. Make sure to use `pow()` instead for exponentiation. In case of performing a XOR operation, use `.bitxor()` for clarity. + +## Exploit Scenario + +In the following example, the `^` operand is being used for exponentiation. But in Rust, `^` is the operand for an XOR operation. If misused, this could lead to unexpected behaviour in our contract. + +```rust + pub fn exp_data_3(e: Env) -> u128 { + let mut data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + + data = data ^ 3; + return data; + } +``` + +The vulnerable code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example). + +## Remediation + +A possible solution is to use the method `pow()`. But, if a XOR operation is wanted, `.bitxor()` method is recommended. + +```rust + pub fn exp_data_3(e: Env) -> u128 { + let data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + + return data.pow(3); + } +``` + +The remediated code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example). + +## References + +- https://doc.rust-lang.org/std/ops/trait.BitXor.html + From b9369b325899606e9eb381725a0546c4513a07d6 Mon Sep 17 00:00:00 2001 From: tomasavola Date: Mon, 6 May 2024 11:43:34 -0300 Subject: [PATCH 04/21] Add incorrect-exponentation in the detector's README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 1792d107..de78bdaf 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ Currently Scout includes the following detectors. | [unrestricted-transfer-from](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unrestricted-transfer-from) | Avoid passing an user-defined parameter as a `from` field in transfer-from. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unrestricted-transfer-from/unrestricted-transfer-from-1) | Critical | | [unsafe-map-get](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-map-get) | Inappropriate usage of the `get` method for `Map` in soroban | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1) | Medium | | [zero-or-test-address](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address) | Avoid zero or test address assignment to prevent contract control loss. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/zero-or-test-address/zero-or-test-address-1) | Validations and error handling | +| [incorrect-exponentation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation) | Warns against incorrect usage of ´^´. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1) | Critical | ## Output formats @@ -133,3 +134,4 @@ Our team has an academic background in computer science and mathematics, with wo ## License Scout is licensed and distributed under a MIT license. [Contact us](https://www.coinfabrik.com/) if you're looking for an exception to the terms. + From ff67856deb6a9ce3e07347dcdaa261ad4617a1e0 Mon Sep 17 00:00:00 2001 From: Nacho Gutman Date: Mon, 6 May 2024 11:47:09 -0300 Subject: [PATCH 05/21] Add incorrect exponentiation to vulnerabilities readme --- docs/docs/vulnerabilities/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/docs/vulnerabilities/README.md b/docs/docs/vulnerabilities/README.md index 06feecc3..be0fe933 100644 --- a/docs/docs/vulnerabilities/README.md +++ b/docs/docs/vulnerabilities/README.md @@ -257,3 +257,9 @@ Assigning a test address can also have similar implications, including the loss This vulnerability falls under the [Validations and error handling](#vulnerability-categories) category and has a Medium severity. + +### Incorrect Exponentiation + +It's common to use `^` for exponentiation. However in Rust, `^` is the XOR operator. If the `^` operator is used, it could lead to unexpected behavior in the contract. It's recommended to use the method `pow()` for exponentiation or `.bitxor()` for XOR operations. + +This vulnerability falls under the [Arithmetic](#vulnerability-categories) category and has a Critical severity. \ No newline at end of file From 931b6147cd04b151ec74f7f954172d6788fa155b Mon Sep 17 00:00:00 2001 From: tomasavola Date: Mon, 6 May 2024 11:50:14 -0300 Subject: [PATCH 06/21] Add 21-incorrect-exponentiation.md documentation --- .../detectors/21-incorrect-exponentiation.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 docs/docs/detectors/21-incorrect-exponentiation.md diff --git a/docs/docs/detectors/21-incorrect-exponentiation.md b/docs/docs/detectors/21-incorrect-exponentiation.md new file mode 100644 index 00000000..b3b715bf --- /dev/null +++ b/docs/docs/detectors/21-incorrect-exponentiation.md @@ -0,0 +1,43 @@ +# Zero or test address + +### What it does +Checks whether the zero address is being inputed to a function without validation. + +### Why is this bad? +Because the private key for the zero address is known, anyone could take ownership of the contract. + +### Example + +```rust +pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> { + if !ZeroAddressContract::ensure_is_admin(&e, admin)? { + return Err(Error::NotAdmin); + } + e.storage().persistent().set(&DataKey::Data, &data); + Ok(()) +} +``` + + +Use instead: +```rust +pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> { + if admin + == Address::from_string(&String::from_bytes( + &e, + b"GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + )) + { + return Err(Error::InvalidNewAdmin); + } + if !ZeroAddressContract::ensure_is_admin(&e, admin)? { + return Err(Error::NotAdmin); + } + e.storage().persistent().set(&DataKey::Data, &data); + Ok(()) +} +``` + +### Implementation + +The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address). From 566eca503d9daca14f6bdd48ebd0ab1107ee5854 Mon Sep 17 00:00:00 2001 From: nachogutman <108377975+nachogutman@users.noreply.github.com> Date: Mon, 6 May 2024 12:25:45 -0300 Subject: [PATCH 07/21] Correct remediated example for clippy --- .../incorrect-exponentiation-1/remediated-example/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index c1554c4d..06dca809 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -32,7 +32,7 @@ impl IncorrectExponentiation { .get::(&DataKey::Data) .expect("Data not found"); - return data.pow(3); + data.pow(3) } } @@ -49,7 +49,7 @@ mod tests { let contract_id = env.register_contract(None, IncorrectExponentiation); let client = IncorrectExponentiationClient::new(&env, &contract_id); env.mock_all_auths(); - let user =
::generate(&env); + let _user =
::generate(&env); client.init(); From 3dfef01b52806ad10588350959f0a5f19bfd39c8 Mon Sep 17 00:00:00 2001 From: nachogutman <108377975+nachogutman@users.noreply.github.com> Date: Mon, 6 May 2024 12:26:26 -0300 Subject: [PATCH 08/21] Correct vulnerable example for clippy --- .../incorrect-exponentiation-1/vulnerable-example/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index c2259eff..9aa96a32 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -33,7 +33,7 @@ impl IncorrectExponentiation { .expect("Data not found"); data = data ^ 3; - return data; + data } } @@ -50,7 +50,7 @@ mod tests { let contract_id = env.register_contract(None, IncorrectExponentiation); let client = IncorrectExponentiationClient::new(&env, &contract_id); env.mock_all_auths(); - let user =
::generate(&env); + let _user =
::generate(&env); client.init(); From 804b31debb5225f9fc3784fdf003640ae786bdaf Mon Sep 17 00:00:00 2001 From: nachogutman <108377975+nachogutman@users.noreply.github.com> Date: Tue, 7 May 2024 09:42:20 -0300 Subject: [PATCH 09/21] Update vulnerable test --- .../incorrect-exponentiation-1/vulnerable-example/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index 9aa96a32..e4df88fb 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -56,7 +56,6 @@ mod tests { client.init(); client.set_data(&10_u128); - //EXPECTED TO FAIL TO SHOW THAT ^ IS NOT .pow() - assert_eq!(client.exp_data_3(), 1000); + assert_eq!(client.exp_data_3(), 9); } } From ebf9d990a3fbe1212bab548294425737278e604d Mon Sep 17 00:00:00 2001 From: tomasavola Date: Tue, 7 May 2024 10:24:18 -0300 Subject: [PATCH 10/21] Add incorrect-exponentiation documentation --- .../detectors/21-incorrect-exponentiation.md | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/docs/docs/detectors/21-incorrect-exponentiation.md b/docs/docs/detectors/21-incorrect-exponentiation.md index b3b715bf..3186fd9a 100644 --- a/docs/docs/detectors/21-incorrect-exponentiation.md +++ b/docs/docs/detectors/21-incorrect-exponentiation.md @@ -1,43 +1,41 @@ -# Zero or test address +# Incorrect Exponentiation ### What it does -Checks whether the zero address is being inputed to a function without validation. + +Warns about `^` being a `bit XOR` operation instead of an exponentiation. ### Why is this bad? -Because the private key for the zero address is known, anyone could take ownership of the contract. + +It can introduce unexpected behaviour in the smart contract. + +#### More info + +- https://doc.rust-lang.org/std/ops/trait.BitXor.html#tymethod.bitxor ### Example ```rust -pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> { - if !ZeroAddressContract::ensure_is_admin(&e, admin)? { - return Err(Error::NotAdmin); + pub fn exp_data_3(e: Env) -> u128 { + let mut data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + data = data ^ 3; + return data; } - e.storage().persistent().set(&DataKey::Data, &data); - Ok(()) -} ``` - - Use instead: + ```rust -pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> { - if admin - == Address::from_string(&String::from_bytes( - &e, - b"GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", - )) - { - return Err(Error::InvalidNewAdmin); - } - if !ZeroAddressContract::ensure_is_admin(&e, admin)? { - return Err(Error::NotAdmin); + pub fn exp_data_3(e: Env) -> u128 { + let data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + return data.pow(3); } - e.storage().persistent().set(&DataKey::Data, &data); - Ok(()) -} ``` ### Implementation -The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address). +The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation). From 1346ba52a27ca35892434557f9d08dec9329d788 Mon Sep 17 00:00:00 2001 From: nachogutman <108377975+nachogutman@users.noreply.github.com> Date: Tue, 7 May 2024 11:30:13 -0300 Subject: [PATCH 11/21] Correct vulnerable testcase for clippy to ignore data = data ^ 3 --- .../incorrect-exponentiation-1/vulnerable-example/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index e4df88fb..c9af8a1c 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -1,4 +1,5 @@ #![no_std] +#![allow(clippy::assign_op_pattern)] use soroban_sdk::{contract, contractimpl, contracttype, Env}; From ee79f18073bcb04f785c7f9e828ca67d8971b369 Mon Sep 17 00:00:00 2001 From: nachogutman <108377975+nachogutman@users.noreply.github.com> Date: Tue, 7 May 2024 11:55:31 -0300 Subject: [PATCH 12/21] Try to fix workflow issue --- .github/workflows/test-detectors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index e14c388f..110d2bb8 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -124,6 +124,7 @@ jobs: run: | rustup toolchain install nightly-2023-12-16-x86_64-apple-darwin rustup component add rust-src --toolchain nightly-2023-12-16-x86_64-apple-darwin + rustup component add rust-src --toolchain nightly-2023-12-16-x86_64-unknown-linux-gnu python scripts/run-tests.py --detector=${{ matrix.detector }} comment-on-pr: From e2c447f4496c5f81565140888a90de2454c40741 Mon Sep 17 00:00:00 2001 From: Jose Garcia Crosta Date: Tue, 7 May 2024 15:16:10 -0300 Subject: [PATCH 13/21] Fix CI --- .github/workflows/test-detectors.yml | 18 +++++++----------- scripts/run-fmt.py | 4 ---- scripts/utils.py | 2 +- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index 110d2bb8..906e0261 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -57,8 +57,10 @@ jobs: key: ${{ runner.os }}-tests-${{ hashFiles('**/Cargo.lock') }} restore-keys: ${{ runner.os }}-tests- - - name: Install Rust nightly - run: rustup install nightly-2023-12-16 + - name: Install Rust nightly and add rust-src + run: | + rustup install nightly-2023-12-16 + rustup component add rust-src --toolchain nightly-2023-12-16 - name: Install dylint, dylint-link and cargo-scout-audit run: cargo +nightly-2023-12-16 install cargo-dylint dylint-link cargo-scout-audit @@ -121,11 +123,7 @@ jobs: restore-keys: ${{ runner.os }}-tests- - name: Run unit and integration tests - run: | - rustup toolchain install nightly-2023-12-16-x86_64-apple-darwin - rustup component add rust-src --toolchain nightly-2023-12-16-x86_64-apple-darwin - rustup component add rust-src --toolchain nightly-2023-12-16-x86_64-unknown-linux-gnu - python scripts/run-tests.py --detector=${{ matrix.detector }} + run: python scripts/run-tests.py --detector=${{ matrix.detector }} comment-on-pr: name: Comment on PR @@ -140,13 +138,11 @@ jobs: id: ubuntu_status working-directory: build-status-ubuntu-latest run: echo "status=$(cat status-ubuntu-latest.txt)" >> $GITHUB_OUTPUT - continue-on-error: true - name: Read macOS build status id: macos_status - working-directory: build-status-macos-latest - run: echo "status=$(cat status-macos-latest.txt)" >> $GITHUB_OUTPUT - continue-on-error: true + working-directory: build-status-macos-13 + run: echo "status=$(cat status-macos-13.txt)" >> $GITHUB_OUTPUT - name: Find comment id: find_comment diff --git a/scripts/run-fmt.py b/scripts/run-fmt.py index 7685aaaf..824812bd 100644 --- a/scripts/run-fmt.py +++ b/scripts/run-fmt.py @@ -21,10 +21,6 @@ def run_fmt(directories): for root, _, _ in os.walk(directory): if is_rust_project(root): start_time = time.time() - returncode, _, stderr = run_subprocess( - ["cargo", "fmt"], - cwd=root, - ) returncode, _, stderr = run_subprocess( ["cargo", "fmt", "--check"], cwd=root, diff --git a/scripts/utils.py b/scripts/utils.py index b9d0e8cf..ffa7008d 100644 --- a/scripts/utils.py +++ b/scripts/utils.py @@ -78,7 +78,7 @@ def print_results(returncode, error_message, check_type, root, elapsed_time): ) if returncode != 0: print(f"\n{RED}{check_type.capitalize()} {issue_type} found in: {root}{ENDC}\n") - if not error_message is None: + if error_message is not None: for line in error_message.strip().split("\n"): print(f"| {line}") print("\n") From 119b5ed671ad9537610f70a014980fc0bf629142 Mon Sep 17 00:00:00 2001 From: Jose Garcia Crosta Date: Tue, 7 May 2024 15:25:12 -0300 Subject: [PATCH 14/21] Delete rustfmt unused config file and fix rust-fmt ci --- rustfmt.toml | 3 -- .../incorrect-exponentiation/Cargo.toml | 15 ++++--- .../remediated-example/Cargo.toml | 2 +- .../remediated-example/src/lib.rs | 42 +++++++++---------- .../vulnerable-example/Cargo.toml | 2 +- .../vulnerable-example/src/lib.rs | 42 +++++++++---------- 6 files changed, 48 insertions(+), 58 deletions(-) delete mode 100644 rustfmt.toml diff --git a/rustfmt.toml b/rustfmt.toml deleted file mode 100644 index cc30efb4..00000000 --- a/rustfmt.toml +++ /dev/null @@ -1,3 +0,0 @@ -group_imports = "StdExternalCrate" -ignore = ["scout-audit-clippy-utils"] -unstable_features = true diff --git a/test-cases/incorrect-exponentiation/Cargo.toml b/test-cases/incorrect-exponentiation/Cargo.toml index 3db3ca7b..fc211bbd 100644 --- a/test-cases/incorrect-exponentiation/Cargo.toml +++ b/test-cases/incorrect-exponentiation/Cargo.toml @@ -4,19 +4,18 @@ members = ["incorrect-exponentiation-*/*"] resolver = "2" [workspace.dependencies] -soroban-sdk = "20.3.2" +soroban-sdk = { version = "20.3.2" } [profile.release] -opt-level = "z" -overflow-checks = true +codegen-units = 1 debug = 0 -strip = "symbols" debug-assertions = false -panic = "abort" -codegen-units = 1 lto = true +opt-level = "z" +overflow-checks = true +panic = "abort" +strip = "symbols" -# For more information about this profile see https://soroban.stellar.org/docs/basic-tutorials/logging#cargotoml-profile [profile.release-with-logs] -inherits = "release" debug-assertions = true +inherits = "release" diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml index ee9f0b0a..172c22cd 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "incorrect-exponentiation-remediated-1" -version = "0.0.0" +version = "0.1.0" edition = "2021" [lib] diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 06dca809..45991cb9 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -13,28 +13,27 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - - pub fn init(e: Env){ + pub fn init(e: Env) { e.storage() - .instance() - .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); + .instance() + .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } pub fn set_data(e: Env, new_data: u128) { e.storage() - .instance() - .set::(&DataKey::Data, &new_data); + .instance() + .set::(&DataKey::Data, &new_data); } pub fn exp_data_3(e: Env) -> u128 { - let data = e.storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); + let data = e + .storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); data.pow(3) } - } #[cfg(test)] @@ -45,18 +44,15 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); + client.init(); + client.set_data(&10_u128); - client.init(); - client.set_data(&10_u128); - - assert_eq!(client.exp_data_3(), 1000); -} + assert_eq!(client.exp_data_3(), 1000); + } } - - diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml index 0300c6ee..9c2ff3ae 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "incorrect-exponentiation-vulnerable-1" -version = "0.0.0" +version = "0.1.0" edition = "2021" [lib] diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index c9af8a1c..94e7f8a3 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -14,29 +14,28 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - - pub fn init(e: Env){ + pub fn init(e: Env) { e.storage() - .instance() - .set::(&DataKey::Data, &((255_u128^2) - 1)); + .instance() + .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); } pub fn set_data(e: Env, new_data: u128) { e.storage() - .instance() - .set::(&DataKey::Data, &new_data); + .instance() + .set::(&DataKey::Data, &new_data); } pub fn exp_data_3(e: Env) -> u128 { - let mut data = e.storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - + let mut data = e + .storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + data = data ^ 3; data } - } #[cfg(test)] @@ -47,16 +46,15 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); + client.init(); + client.set_data(&10_u128); - client.init(); - client.set_data(&10_u128); - - assert_eq!(client.exp_data_3(), 9); -} + assert_eq!(client.exp_data_3(), 9); + } } From 21d4f442166ff9c5403cb08bac79cf711d6ed4f1 Mon Sep 17 00:00:00 2001 From: Jose Garcia Crosta Date: Tue, 7 May 2024 16:28:38 -0300 Subject: [PATCH 15/21] Add manual dispatch on workflows --- .github/workflows/deploy-docs.yml | 1 + .github/workflows/general-rust.yml | 1 + .github/workflows/test-deploy-docs.yml | 1 + .github/workflows/test-detectors.yml | 1 + 4 files changed, 4 insertions(+) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 38253d87..bed58610 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -6,6 +6,7 @@ on: - main paths: - "docs/**" + workflow_dispatch: jobs: deploy: diff --git a/.github/workflows/general-rust.yml b/.github/workflows/general-rust.yml index d8de7de7..c8b0e93d 100644 --- a/.github/workflows/general-rust.yml +++ b/.github/workflows/general-rust.yml @@ -8,6 +8,7 @@ on: - "scripts/**" - "!detectors/**/*.md" - "!test-cases/**/*.md" + workflow_dispatch: env: CARGO_TERM_COLOR: always diff --git a/.github/workflows/test-deploy-docs.yml b/.github/workflows/test-deploy-docs.yml index 80afe1ad..7fe2e13e 100644 --- a/.github/workflows/test-deploy-docs.yml +++ b/.github/workflows/test-deploy-docs.yml @@ -6,6 +6,7 @@ on: - main paths: - "docs/**" + workflow_dispatch: jobs: test-deploy: diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index 906e0261..eb8f3085 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -8,6 +8,7 @@ on: - "scripts/**" - "!detectors/**/*.md" - "!test-cases/**/*.md" + workflow_dispatch: env: CARGO_TERM_COLOR: always From a74674008d894892aab24439c6ecd93ec4bb8561 Mon Sep 17 00:00:00 2001 From: tomasavola Date: Thu, 9 May 2024 11:28:15 -0300 Subject: [PATCH 16/21] Update vulnerability --- .../21-incorrect-exponentiation.md | 6 +-- .../remediated-example/src/lib.rs | 40 ++++++++-------- .../vulnerable-example/src/lib.rs | 46 ++++++++----------- 3 files changed, 42 insertions(+), 50 deletions(-) diff --git a/docs/docs/vulnerabilities/21-incorrect-exponentiation.md b/docs/docs/vulnerabilities/21-incorrect-exponentiation.md index 4e9ef330..44946f76 100644 --- a/docs/docs/vulnerabilities/21-incorrect-exponentiation.md +++ b/docs/docs/vulnerabilities/21-incorrect-exponentiation.md @@ -21,8 +21,8 @@ In the following example, the `^` operand is being used for exponentiation. But .get::(&DataKey::Data) .expect("Data not found"); - data = data ^ 3; - return data; + data ^= 3; + data } ``` @@ -39,7 +39,7 @@ A possible solution is to use the method `pow()`. But, if a XOR operation is wan .get::(&DataKey::Data) .expect("Data not found"); - return data.pow(3); + data.pow(3) } ``` diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 45991cb9..53d48a5b 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -13,27 +13,22 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - pub fn init(e: Env) { - e.storage() - .instance() - .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); - } pub fn set_data(e: Env, new_data: u128) { e.storage() - .instance() - .set::(&DataKey::Data, &new_data); + .instance() + .set::(&DataKey::Data, &new_data); } pub fn exp_data_3(e: Env) -> u128 { - let data = e - .storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); + let data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); data.pow(3) } + } #[cfg(test)] @@ -44,15 +39,18 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); - client.init(); - client.set_data(&10_u128); - assert_eq!(client.exp_data_3(), 1000); - } + + client.set_data(&10_u128); + + assert_eq!(client.exp_data_3(), 1000); +} } + + diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index 94e7f8a3..b6c746c9 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -1,5 +1,4 @@ #![no_std] -#![allow(clippy::assign_op_pattern)] use soroban_sdk::{contract, contractimpl, contracttype, Env}; @@ -14,28 +13,24 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - pub fn init(e: Env) { - e.storage() - .instance() - .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); - } pub fn set_data(e: Env, new_data: u128) { e.storage() - .instance() - .set::(&DataKey::Data, &new_data); + .instance() + .set::(&DataKey::Data, &new_data); } + pub fn exp_data_3(e: Env) -> u128 { - let mut data = e - .storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - - data = data ^ 3; + let mut data = e.storage() + .instance() + .get::(&DataKey::Data) + .expect("Data not found"); + + data ^= 3; data } + } #[cfg(test)] @@ -46,15 +41,14 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); - - client.init(); - client.set_data(&10_u128); - - assert_eq!(client.exp_data_3(), 9); - } + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); + + client.set_data(&10_u128); + + assert_eq!(client.exp_data_3(), 9); +} } From 10e840205c1153e60ba15bc65adcb7534dbc9071 Mon Sep 17 00:00:00 2001 From: user Date: Thu, 9 May 2024 12:54:21 -0300 Subject: [PATCH 17/21] Correct test cases and detector documentation --- .../detectors/21-incorrect-exponentiation.md | 21 +++++------ .../remediated-example/src/lib.rs | 33 +++++++++-------- .../vulnerable-example/src/lib.rs | 36 +++++++++---------- 3 files changed, 41 insertions(+), 49 deletions(-) diff --git a/docs/docs/detectors/21-incorrect-exponentiation.md b/docs/docs/detectors/21-incorrect-exponentiation.md index 3186fd9a..b2584959 100644 --- a/docs/docs/detectors/21-incorrect-exponentiation.md +++ b/docs/docs/detectors/21-incorrect-exponentiation.md @@ -15,24 +15,19 @@ It can introduce unexpected behaviour in the smart contract. ### Example ```rust - pub fn exp_data_3(e: Env) -> u128 { - let mut data = e.storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - data = data ^ 3; - return data; + pub fn init(e: Env){ + e.storage() + .instance() + .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); } ``` Use instead: ```rust - pub fn exp_data_3(e: Env) -> u128 { - let data = e.storage() - .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - return data.pow(3); + pub fn init(e: Env) { + e.storage() + .instance() + .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } ``` diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 45991cb9..22c8ab10 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -1,6 +1,13 @@ #![no_std] -use soroban_sdk::{contract, contractimpl, contracttype, Env}; +use soroban_sdk::{contract, contractimpl, contracttype, contracterror, Env}; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum IEError { + CouldntRetrieveData = 1, +} #[contracttype] #[derive(Clone)] @@ -19,20 +26,14 @@ impl IncorrectExponentiation { .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } - pub fn set_data(e: Env, new_data: u128) { - e.storage() - .instance() - .set::(&DataKey::Data, &new_data); - } - - pub fn exp_data_3(e: Env) -> u128 { - let data = e - .storage() + pub fn get_data(e: Env) -> Result { + let data = e.storage() .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - - data.pow(3) + .get(&DataKey::Data); + match data { + Some(x) => Ok(x), + None => return Err(IEError::CouldntRetrieveData) + } } } @@ -51,8 +52,6 @@ mod tests { let _user =
::generate(&env); client.init(); - client.set_data(&10_u128); - - assert_eq!(client.exp_data_3(), 1000); + assert_eq!(client.get_data(), 65024); } } diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index 94e7f8a3..13d3af8d 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -1,7 +1,14 @@ #![no_std] #![allow(clippy::assign_op_pattern)] -use soroban_sdk::{contract, contractimpl, contracttype, Env}; +use soroban_sdk::{contract, contractimpl, contracttype, contracterror, Env}; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum IEError { + CouldntRetrieveData = 1, +} #[contracttype] #[derive(Clone)] @@ -14,27 +21,20 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - pub fn init(e: Env) { + pub fn init(e: Env){ e.storage() .instance() .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); } - pub fn set_data(e: Env, new_data: u128) { - e.storage() - .instance() - .set::(&DataKey::Data, &new_data); - } - - pub fn exp_data_3(e: Env) -> u128 { - let mut data = e - .storage() + pub fn get_data(e: Env) -> Result { + let data = e.storage() .instance() - .get::(&DataKey::Data) - .expect("Data not found"); - - data = data ^ 3; - data + .get(&DataKey::Data); + match data { + Some(x) => Ok(x), + None => return Err(IEError::CouldntRetrieveData) + } } } @@ -53,8 +53,6 @@ mod tests { let _user =
::generate(&env); client.init(); - client.set_data(&10_u128); - - assert_eq!(client.exp_data_3(), 9); + assert_ne!(client.get_data(), 65024); } } From 21afdcbc48bb13e2454e5af0a83a8abfc16c595a Mon Sep 17 00:00:00 2001 From: arturoBeccar Date: Thu, 9 May 2024 13:51:36 -0300 Subject: [PATCH 18/21] Use pow() --- .../incorrect-exponentiation-1/remediated-example/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 437923fe..947ace1a 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -23,7 +23,7 @@ impl IncorrectExponentiation { pub fn init(e: Env){ e.storage() .instance() - .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); + .set::(&DataKey::Data, &((255_u128.pow(2)) - 1)); } pub fn get_data(e: Env) -> Result { From 9e774b6a219259c061e480accb8e25e70701b6d5 Mon Sep 17 00:00:00 2001 From: arturoBeccar Date: Thu, 9 May 2024 13:53:12 -0300 Subject: [PATCH 19/21] Remove parenthesis --- .../incorrect-exponentiation-1/remediated-example/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 947ace1a..a8d82fb7 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -23,7 +23,7 @@ impl IncorrectExponentiation { pub fn init(e: Env){ e.storage() .instance() - .set::(&DataKey::Data, &((255_u128.pow(2)) - 1)); + .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } pub fn get_data(e: Env) -> Result { @@ -35,7 +35,6 @@ impl IncorrectExponentiation { None => return Err(IEError::CouldntRetrieveData) } } - } #[cfg(test)] From b5817a2e9065118c421c6117abf089b44913db3e Mon Sep 17 00:00:00 2001 From: user Date: Fri, 10 May 2024 09:52:22 -0300 Subject: [PATCH 20/21] Correct format and unneeded return --- .../remediated-example/src/lib.rs | 20 +++++++++---------- .../vulnerable-example/src/lib.rs | 20 +++++++++---------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index a8d82fb7..7b0a713f 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -use soroban_sdk::{contract, contractimpl, contracttype, contracterror, Env}; +use soroban_sdk::{contract, contracterror, contractimpl, contracttype, Env}; #[contracterror] #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] @@ -20,19 +20,17 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - pub fn init(e: Env){ + pub fn init(e: Env) { e.storage() .instance() .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } pub fn get_data(e: Env) -> Result { - let data = e.storage() - .instance() - .get(&DataKey::Data); + let data = e.storage().instance().get(&DataKey::Data); match data { Some(x) => Ok(x), - None => return Err(IEError::CouldntRetrieveData) + None => Err(IEError::CouldntRetrieveData), } } } @@ -45,11 +43,11 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); client.init(); assert_eq!(client.get_data(), 65024); diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index 3c4e4839..d1fdec44 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -use soroban_sdk::{contract, contractimpl, contracttype, contracterror, Env}; +use soroban_sdk::{contract, contracterror, contractimpl, contracttype, Env}; #[contracterror] #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] @@ -20,19 +20,17 @@ pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - pub fn init(e: Env){ + pub fn init(e: Env) { e.storage() .instance() .set::(&DataKey::Data, &((255_u128 ^ 2) - 1)); } pub fn get_data(e: Env) -> Result { - let data = e.storage() - .instance() - .get(&DataKey::Data); + let data = e.storage().instance().get(&DataKey::Data); match data { Some(x) => Ok(x), - None => return Err(IEError::CouldntRetrieveData) + None => Err(IEError::CouldntRetrieveData), } } } @@ -45,11 +43,11 @@ mod tests { #[test] fn simple_test() { - let env = Env::default(); - let contract_id = env.register_contract(None, IncorrectExponentiation); - let client = IncorrectExponentiationClient::new(&env, &contract_id); - env.mock_all_auths(); - let _user =
::generate(&env); + let env = Env::default(); + let contract_id = env.register_contract(None, IncorrectExponentiation); + let client = IncorrectExponentiationClient::new(&env, &contract_id); + env.mock_all_auths(); + let _user =
::generate(&env); client.init(); assert_ne!(client.get_data(), 65024); From d74858ebd1834882cfd14db3a33a8ec3a7012215 Mon Sep 17 00:00:00 2001 From: user Date: Fri, 10 May 2024 10:14:38 -0300 Subject: [PATCH 21/21] correct format --- .../incorrect-exponentiation-1/remediated-example/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs index 7b0a713f..9db61a4b 100644 --- a/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs +++ b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/src/lib.rs @@ -25,7 +25,7 @@ impl IncorrectExponentiation { .instance() .set::(&DataKey::Data, &(255_u128.pow(2) - 1)); } - + pub fn get_data(e: Env) -> Result { let data = e.storage().instance().get(&DataKey::Data); match data { @@ -53,5 +53,3 @@ mod tests { assert_eq!(client.get_data(), 65024); } } - -