From ce5b8aaf82ae554e20d3c6b364d82089700947ec Mon Sep 17 00:00:00 2001 From: user Date: Thu, 9 May 2024 09:23:37 -0300 Subject: [PATCH 1/5] Upgraded detector --- detectors/incorrect-exponentiation/Cargo.toml | 16 ++++ detectors/incorrect-exponentiation/src/lib.rs | 89 +++++++++++++++++++ 2 files changed, 105 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..ab0e3dfb --- /dev/null +++ b/detectors/incorrect-exponentiation/src/lib.rs @@ -0,0 +1,89 @@ +#![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::AssignOp(binop, _, _) = &expr.kind { + if binop.node == rustc_hir::BinOpKind::BitXor{ + self.incorrect_exponentiation = true; + self.span.push(expr.span); + } + } + + 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 3bcf0ff274815f2d688692cf231dcf564273c16c Mon Sep 17 00:00:00 2001 From: tomasavola Date: Thu, 9 May 2024 09:49:12 -0300 Subject: [PATCH 2/5] new test case --- .../remediated-example/Cargo.toml | 16 +++++ .../remediated-example/src/lib.rs | 62 +++++++++++++++++++ .../vulnerable-example/Cargo.toml | 16 +++++ .../vulnerable-example/src/lib.rs | 62 +++++++++++++++++++ 4 files changed, 156 insertions(+) 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/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..06dca809 --- /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"); + + 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..08ccc0bb --- /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 ^= 3; + 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); + + assert_eq!(client.exp_data_3(), 9); +} +} From 2f381a7782eda05f178fd2da6206d29484f31c03 Mon Sep 17 00:00:00 2001 From: tomasavola Date: Thu, 9 May 2024 11:20:01 -0300 Subject: [PATCH 3/5] Delete fn init() --- .../remediated-example/src/lib.rs | 8 +------- .../vulnerable-example/src/lib.rs | 8 -------- 2 files changed, 1 insertion(+), 15 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 06dca809..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 @@ -14,12 +14,6 @@ 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() @@ -52,7 +46,7 @@ mod tests { 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/src/lib.rs b/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example/src/lib.rs index 08ccc0bb..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 @@ -14,12 +14,6 @@ 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() @@ -53,8 +47,6 @@ mod tests { env.mock_all_auths(); let _user =
::generate(&env); - - client.init(); client.set_data(&10_u128); assert_eq!(client.exp_data_3(), 9); From 611ffa47533756839767268acfe50028f0b7e10a Mon Sep 17 00:00:00 2001 From: tomasavola Date: Fri, 10 May 2024 09:52:23 -0300 Subject: [PATCH 4/5] Improved tests --- .../vulnerable-example/Cargo.toml | 6 +----- .../vulnerable-example/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) 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..4b9c72d1 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,16 +1,12 @@ [package] name = "incorrect-exponentiation-vulnerable-1" -version = "0.0.0" +version = "0.1.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 index b6c746c9..c5d98c8c 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 @@ -47,8 +47,8 @@ mod tests { env.mock_all_auths(); let _user =
::generate(&env); - client.set_data(&10_u128); + client.set_data(&3_u128); - assert_eq!(client.exp_data_3(), 9); + assert_ne!(client.exp_data_3(), 27); } } From afd390aee313bf96e0ec1dc09ce6eb7118610641 Mon Sep 17 00:00:00 2001 From: tomasavola Date: Fri, 10 May 2024 11:06:37 -0300 Subject: [PATCH 5/5] Delete expect warning --- .../incorrect-exponentiation/Cargo.toml | 18 +++++++++++ .../remediated-example/Cargo.toml | 6 +--- .../remediated-example/src/lib.rs | 32 +++++++++++-------- .../vulnerable-example/src/lib.rs | 29 ++++++++++------- 4 files changed, 55 insertions(+), 30 deletions(-) create mode 100644 test-cases/incorrect-exponentiation/Cargo.toml diff --git a/test-cases/incorrect-exponentiation/Cargo.toml b/test-cases/incorrect-exponentiation/Cargo.toml new file mode 100644 index 00000000..24be4479 --- /dev/null +++ b/test-cases/incorrect-exponentiation/Cargo.toml @@ -0,0 +1,18 @@ +[workspace] +exclude = [".cargo", "target"] +members = ["incorrect-exponentiation-*/*"] +resolver = "2" +[workspace.dependencies] +soroban-sdk = { version = "20.3.2" } +[profile.release] +codegen-units = 1 +debug = 0 +debug-assertions = false +lto = true +opt-level = "z" +overflow-checks = true +panic = "abort" +strip = "symbols" +[profile.release-with-logs] +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..1c7d3486 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,16 +1,12 @@ [package] name = "incorrect-exponentiation-remediated-1" -version = "0.0.0" +version = "0.1.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 index 53d48a5b..323aad7c 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, Env}; +use soroban_sdk::{contract, contractimpl,contracterror, contracttype, Env}; #[contracttype] #[derive(Clone)] @@ -8,27 +8,33 @@ enum DataKey { Data, } +// Agrega el atributo #[contracterror] a la definición de IEError +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum IEError { + CouldntRetrieveData = 1, +} + #[contract] pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - 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"); - - data.pow(3) + pub fn exp_data_3(e: Env) -> Result { + let data:Option = e.storage().instance().get(&DataKey::Data); + match data + { + Some(x) => return Ok(x.pow(3)), + None =>return Err(IEError::CouldntRetrieveData), + } } - } #[cfg(test)] @@ -47,10 +53,8 @@ mod tests { - client.set_data(&10_u128); + client.set_data(&3_u128); - assert_eq!(client.exp_data_3(), 1000); + assert_eq!(client.exp_data_3(), 27); } } - - 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 c5d98c8c..dbe8ee79 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, Env}; +use soroban_sdk::{contract, contractimpl,contracterror, contracttype, Env}; #[contracttype] #[derive(Clone)] @@ -8,27 +8,34 @@ enum DataKey { Data, } +// Agrega el atributo #[contracterror] a la definición de IEError +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum IEError { + CouldntRetrieveData = 1, +} + #[contract] pub struct IncorrectExponentiation; #[contractimpl] impl IncorrectExponentiation { - 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 ^= 3; - data + pub fn exp_data_3(e: Env) -> Result { + let data: Option = e.storage().instance().get(&DataKey::Data); + match data { + Some(mut x) => { + x ^= 3; + Ok(x) + }, + None => Err(IEError::CouldntRetrieveData), + } } }