diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index 81c7a22a..0ad5d671 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -119,9 +119,9 @@ jobs: with: path: ~/.cargo key: ${{ runner.os }}-tests-${{ hashFiles('**/Cargo.lock') }}. -# This is broken until ink! solves stdsimd problem. -# - name: Run unit and integration tests -# run: python scripts/run-tests.py --detector=${{ matrix.detector }} + # This is broken until ink! solves stdsimd problem. + # - name: Run unit and integration tests + # run: python scripts/run-tests.py --detector=${{ matrix.detector }} comment-on-pr: name: Comment on PR diff --git a/detectors/Cargo.toml b/detectors/Cargo.toml index 28f93052..ee0eb109 100644 --- a/detectors/Cargo.toml +++ b/detectors/Cargo.toml @@ -7,6 +7,6 @@ dylint_testing = "3.0.0" if_chain = "1.0.2" scout-audit-clippy-utils = { version = "=0.2.3" } - -itertools = { version = "0.12" } +scout-audit-internal = { version = "=0.2.3", features = ["detector", "lint_helper"] } dylint_linting = { package = "scout-audit-dylint-linting", version = "3.0.1" } +itertools = { version = "0.12" } diff --git a/detectors/lazy-values-not-set/Cargo.toml b/detectors/lazy-values-not-set/Cargo.toml new file mode 100644 index 00000000..ca415813 --- /dev/null +++ b/detectors/lazy-values-not-set/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "lazy-values-not-set" +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 } + + +[dev-dependencies] +dylint_testing = { workspace = true } + +[package.metadata.rust-analyzer] +rustc_private = true \ No newline at end of file diff --git a/detectors/lazy-values-not-set/src/lib.rs b/detectors/lazy-values-not-set/src/lib.rs new file mode 100644 index 00000000..50eb5d63 --- /dev/null +++ b/detectors/lazy-values-not-set/src/lib.rs @@ -0,0 +1,330 @@ +#![feature(rustc_private)] +#![recursion_limit = "256"] + +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_middle; +extern crate rustc_span; + +use std::collections::HashMap; + +use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; +use rustc_hir::{Body, FnDecl}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::traversal::preorder; +use rustc_middle::mir::{Local, Operand, Rvalue, TerminatorKind}; +use rustc_middle::ty::TyKind; +use rustc_span::def_id::LocalDefId; +use rustc_span::Span; +use scout_audit_clippy_utils::match_def_path; + +const LINT_MESSAGE: &str = "Lazy value was gotten here but never set afterwards"; + +dylint_linting::impl_late_lint! { + pub LAZY_VALUES_NOT_SET, + Warn, + LINT_MESSAGE, + LazyValuesNotSet::default(), + { + name: "Lazy values get and not set", + long_message: "When a get is performed, a copy of the value is received; if that copy is modified, the new value must be set afterwards.", + severity: "Critical", + help: "https://coinfabrik.github.io/scout/docs/vulnerabilities/lazy-values-not-set", + vulnerability_class: "Best practices", + } +} + +#[derive(Default)] +pub struct LazyValuesNotSet { + lazy_set_defid: Option, + lazy_get_defid: Option, + mapping_insert_defid: Option, + mapping_get_defid: Option, +} + +struct FunFinderVisitor<'a, 'tcx: 'a> { + cx: &'a LateContext<'tcx>, + lazy_set_defid: Option, + lazy_get_defid: Option, + mapping_insert_defid: Option, + mapping_get_defid: Option, +} + +impl<'a, 'tcx> Visitor<'tcx> for FunFinderVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { + if let rustc_hir::ExprKind::MethodCall(path, receiver, _, _) = expr.kind { + if path.ident.to_string().contains("get") + || path.ident.to_string().contains("set") + || path.ident.to_string().contains("insert") + { + let defid = self.cx.typeck_results().type_dependent_def_id(expr.hir_id); + + let receiver_type = self.cx.typeck_results().expr_ty(receiver); + if let TyKind::Adt(def, _) = receiver_type.kind() { + if match_def_path(self.cx, def.did(), &["ink_storage", "lazy", "Lazy"]) { + if path.ident.to_string().contains("get") { + self.lazy_get_defid = defid; + } else { + self.lazy_set_defid = defid; + } + } else if match_def_path( + self.cx, + def.did(), + &["ink_storage", "lazy", "mapping", "Mapping"], + ) { + if path.ident.to_string().contains("get") { + self.mapping_get_defid = defid; + } else { + self.mapping_insert_defid = defid; + } + } + } + } + } + walk_expr(self, expr); + } +} + +impl<'tcx> LateLintPass<'tcx> for LazyValuesNotSet { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _: Span, + id: LocalDefId, + ) { + //search for the defids of the different functions + let mut visitor = FunFinderVisitor { + cx, + lazy_set_defid: None, + lazy_get_defid: None, + mapping_insert_defid: None, + mapping_get_defid: None, + }; + visitor.visit_expr(body.value); + if visitor.lazy_set_defid.is_some() { + self.lazy_set_defid = visitor.lazy_set_defid; + } + if visitor.lazy_get_defid.is_some() { + self.lazy_get_defid = visitor.lazy_get_defid; + } + if visitor.mapping_insert_defid.is_some() { + self.mapping_insert_defid = visitor.mapping_insert_defid; + } + if visitor.mapping_get_defid.is_some() { + self.mapping_get_defid = visitor.mapping_get_defid; + } + + let (_, hm) = self.get_func_info(cx, id.to_def_id(), &[], &[], &mut vec![]); + for val in hm.values() { + scout_audit_clippy_utils::diagnostics::span_lint( + cx, + LAZY_VALUES_NOT_SET, + *val, + LINT_MESSAGE, + ); + } + } +} + +fn clean_local_upwards(local: Local, hm: &HashMap>) -> Vec { + let val = hm.get(&local); + let mut ret_vec: Vec = vec![]; + if let Some(locals_vec) = val { + ret_vec.extend(locals_vec); + for local in locals_vec { + ret_vec.extend(clean_local_upwards(*local, hm)) + } + } + ret_vec.dedup(); + ret_vec +} + +impl LazyValuesNotSet { + fn get_func_info( + &mut self, + cx: &LateContext, + func_defid: DefId, + tainted_get_map: &[Local], + tainted_get_lazy: &[Local], + visited_funs: &mut Vec, + ) -> (Vec, HashMap) { + if visited_funs.contains(&func_defid) { + return (vec![], HashMap::new()); + } + visited_funs.push(func_defid); + let mir = cx.tcx.optimized_mir(func_defid); + let mir_preorder = preorder(mir); + let mut mapping_get_tainted_args: Vec = tainted_get_map.to_owned(); + let mut lazy_get_tainted_args: Vec = tainted_get_lazy.to_owned(); + let mut span_local: HashMap = HashMap::new(); + let mut locals_dependencies: HashMap> = HashMap::new(); + let mut locals_to_clean: Vec = vec![]; + for basicblock in mir_preorder { + for stmt in basicblock.1.statements.iter().rev() { + if let rustc_middle::mir::StatementKind::Assign(box_) = &stmt.kind { + let locals = get_locals_in_rvalue(&box_.1); + locals_dependencies.insert(box_.0.local, locals.clone()); + for local in locals { + if mapping_get_tainted_args.contains(&local) { + mapping_get_tainted_args.push(box_.0.local); + } + if lazy_get_tainted_args.contains(&local) { + lazy_get_tainted_args.push(box_.0.local); + } + } + } + } + if let Some(terminator) = &basicblock.1.terminator { + if let TerminatorKind::Call { + func, + args, + destination, + fn_span, + .. + } = &terminator.kind + { + match func { + rustc_middle::mir::Operand::Copy(_) + | rustc_middle::mir::Operand::Move(_) => {} + rustc_middle::mir::Operand::Constant(b) => { + if let TyKind::FnDef(defid, _args) = b.ty().kind() { + //if the function is set or insert taint destinations local + if self.lazy_get_defid.is_some_and(|did| did == *defid) { + lazy_get_tainted_args.push(destination.local); + span_local.insert(destination.local, *fn_span); + } else if self.mapping_get_defid.is_some_and(|did| did == *defid) { + mapping_get_tainted_args.push(destination.local); + span_local.insert(destination.local, *fn_span); + } + //if the function is defined in the local crate call get_func_info recursively + else if defid.is_local() { + //translate from my locals to the locals into the call + let mut mapping_args: Vec = vec![]; + let mut lazy_args: Vec = vec![]; + for arg in args.iter().enumerate() { + match arg.1 { + Operand::Copy(a) | Operand::Move(a) => { + if mapping_get_tainted_args.contains(&a.local) { + mapping_args.push(Local::from_usize(arg.0 + 1)); + } + if lazy_get_tainted_args.contains(&a.local) { + lazy_args.push(Local::from_usize(arg.0 + 1)); + } + } + Operand::Constant(_) => {} + } + } + let cleaned_taints = self.get_func_info( + cx, + *defid, + &mapping_args, + &lazy_args, + visited_funs, + ); + //get the locals translated from call locals + for local in cleaned_taints.0 { + let op_arg = args.get(local.as_usize() - 1); + if let Some(arg) = op_arg { + match arg { + Operand::Copy(a) | Operand::Move(a) => { + //clean the taints + mapping_get_tainted_args + .retain(|i| a.local != *i); + lazy_get_tainted_args.retain(|i| a.local != *i); + //push locals to be cleaned before + locals_to_clean.push(a.local) + } + Operand::Constant(_) => {} + } + } + } + } + //if is an insert call clean the taints upwards + else if self.lazy_set_defid.is_some_and(|did| did == *defid) { + for arg in args { + match arg { + Operand::Copy(a) | Operand::Move(a) => { + locals_to_clean.push(a.local); + } + Operand::Constant(_) => {} + } + } + } else if self.mapping_insert_defid.is_some_and(|did| did == *defid) + { + for arg in args { + match arg { + Operand::Copy(a) | Operand::Move(a) => { + locals_to_clean.push(a.local); + } + Operand::Constant(_) => {} + } + } + } else { + let mut args_locals = vec![]; + for arg in args { + match arg { + Operand::Copy(a) | Operand::Move(a) => { + args_locals.push(a.local); + } + Operand::Constant(_) => {} + } + } + locals_dependencies.insert(destination.local, args_locals); + } + } + } + } + } + } + } + for local in locals_to_clean.clone() { + locals_to_clean.extend(clean_local_upwards(local, &locals_dependencies)); + } + locals_to_clean.dedup(); + for local in &locals_to_clean { + span_local.remove(local); + } + ( + locals_to_clean + .clone() + .into_iter() + .filter(|l| l.as_usize() <= mir.arg_count) + .collect::>(), + span_local, + ) + } +} + +fn get_locals_in_rvalue(rvalue: &Rvalue) -> Vec { + fn op_local(op: &Operand) -> Vec { + match op { + rustc_middle::mir::Operand::Copy(p) | rustc_middle::mir::Operand::Move(p) => { + vec![p.local] + } + rustc_middle::mir::Operand::Constant(_) => vec![], + } + } + match rvalue { + rustc_middle::mir::Rvalue::Use(op) + | rustc_middle::mir::Rvalue::Repeat(op, _) + | rustc_middle::mir::Rvalue::Cast(_, op, _) + | rustc_middle::mir::Rvalue::UnaryOp(_, op) => op_local(op), + rustc_middle::mir::Rvalue::Ref(_, _, p) + | rustc_middle::mir::Rvalue::AddressOf(_, p) + | rustc_middle::mir::Rvalue::Len(p) + | rustc_middle::mir::Rvalue::CopyForDeref(p) => { + vec![p.local] + } + rustc_middle::mir::Rvalue::BinaryOp(_, ops) + | rustc_middle::mir::Rvalue::CheckedBinaryOp(_, ops) => { + let mut v = op_local(&ops.0); + v.extend(op_local(&ops.1)); + v + } + _ => vec![], + } +} diff --git a/docs/docs/detectors/28-lazy-values-not-set.md b/docs/docs/detectors/28-lazy-values-not-set.md new file mode 100644 index 00000000..ec251d86 --- /dev/null +++ b/docs/docs/detectors/28-lazy-values-not-set.md @@ -0,0 +1,55 @@ +# Lazy values get and not set + +### What it does + +Check that if `get` is used on a `Lazy` or `Mapping` variable, `set` or `insert` is subsequently used to change its value. + +### Why is this bad? + +The `get` function of a `Mapping` or a `Lazy` returns a local copy of the value, so changes made to that variable aren't automatically saved in the storage. To save those changes, you must call the `insert` function in the case of a `Mapping`, or `set` in the case of a `Lazy`. + +As indicated [here](https://use.ink/datastructures/mapping#updating-values), it's a common pitfall that we decided to warn about, even though there are cases like getter functions where using "get" is necessary and not modifying the value. For cases like this, you can ignore the lint by adding #[allow(lazy_values_not_set)] immediately before the function definition. + +#### More info + +- https://use.ink/datastructures/mapping#updating-values + +### Example + +```rust + #[ink(message, payable)] + pub fn transfer(&mut self) { + let caller = self.env().caller(); + let mut balance = self.balances.get(caller).unwrap_or(0); + let endowment = self.env().transferred_value(); + balance += endowment; + } +``` + +Use instead: + +```rust + #[ink(message, payable)] + pub fn transfer(&mut self) { + let caller = self.env().caller(); + let mut balance = self.balances.get(caller).unwrap_or(0); + let endowment = self.env().transferred_value(); + balance += endowment; + self.balances.insert(caller, &balance); + } +``` + +If you want to ignore the lint in getter functions do: + +```rust + #[ink(message)] + #[allow(lazy_values_not_set)] + pub fn get_balance(&self) -> Option { + let caller = self.env().caller(); + self.balances.get(caller) + } +``` + +### Implementation + +The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout/tree/main/detectors/lazy-values-not-set). diff --git a/docs/docs/vulnerabilities/28-lazy-values-not-set.md b/docs/docs/vulnerabilities/28-lazy-values-not-set.md new file mode 100644 index 00000000..6b7d0a97 --- /dev/null +++ b/docs/docs/vulnerabilities/28-lazy-values-not-set.md @@ -0,0 +1,59 @@ +# Lazy values not set + +## Description + +- Vulnerability Category: `Best Practices` +- Vulnerability Severity: `Critical` +- Detectors: [`lazy-values-not-set`](https://github.com/CoinFabrik/scout/tree/main/lazy-values-not-set/) +- Test Cases: [`lazy-values-not-set-1`](https://github.com/CoinFabrik/scout/tree/main/test-cases/lazy-values-not-set/lazy-values-not-set-1) + +## Exploit Scenario + +Consider the following contract: + +```rust + #[ink(storage)] + pub struct Contract { + mapping: Mapping, + } + impl Contract { + /* --- snip --- */ + #[ink(message)] + pub fn sum(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let mut _val = self.mapping.get(key).unwrap_or_default(); + _val += value; + Ok(()) + } + /* --- snip --- */ + } +``` + +In this case, when you `.get(...)` a value from a `Lazy` storage field, you _probably_ want to mutate it. The values are not automatically flushed to storage, so you need to `.set(...)` it. + +## Remediation + +Use the `.set(...)` or `.insert(...)` method after using `.get(...)` to flush the new value to storage. + +```rust + #[ink(storage)] + pub struct Contract { + mapping: Mapping, + } + impl Contract { + /* --- snip --- */ + #[ink(message)] + pub fn sum(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let mut _val = self.mapping.get(key).unwrap_or_default(); + _val += value; + self.mapping.insert(key, value); + Ok(()) + } + /* --- snip --- */ + } +``` + +## Known Issues + +If you have a `.get(...)` function that you don't mutate (e.g., used as a const value), this detector triggers, if you want to ignore the lint you could add #[allow(lazy_values_not_set)] immediately before the function definition. diff --git a/scripts/validate-detectors.py b/scripts/validate-detectors.py index bf12634f..8108131f 100644 --- a/scripts/validate-detectors.py +++ b/scripts/validate-detectors.py @@ -156,4 +156,4 @@ def validate_detectors(test_cases_path, detectors_path): print(f"* {error}") exit(1) else: - print(f"{GREEN}\nAll detectors and test cases are valid.{ENDC}") \ No newline at end of file + print(f"{GREEN}\nAll detectors and test cases are valid.{ENDC}") diff --git a/test-cases/lazy-values-not-set/Cargo.toml b/test-cases/lazy-values-not-set/Cargo.toml new file mode 100644 index 00000000..073358d0 --- /dev/null +++ b/test-cases/lazy-values-not-set/Cargo.toml @@ -0,0 +1,26 @@ +[workspace] +exclude = [".cargo", "target"] +members = ["lazy-values-not-set-*/*"] +resolver = "2" + +[workspace.dependencies] +ink = { version = "5.0.0", default-features = false } +scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } +scale-info = { version = "2.6", default-features = false, features = ["derive"]} +ink_e2e = { version = "=5.0.0" } + + +[profile.release] +codegen-units = 1 +debug = 0 +debug-assertions = false +lto = true +opt-level = "z" +overflow-checks = false +panic = "abort" +strip = "symbols" + +[profile.release-with-logs] +debug-assertions = true +inherits = "release" + diff --git a/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/Cargo.toml b/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/Cargo.toml new file mode 100755 index 00000000..d602405f --- /dev/null +++ b/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "lazy-values-not-set-remediated" +version = "0.1.0" +edition = "2021" +authors = ["[your_name] <[your_email]>"] + +[lib] +path = "lib.rs" + +[features] +default = ["std"] +std = [ + "ink/std", + "scale/std", + "scale-info/std", +] +ink-as-dependency = [] +e2e-tests = [] + +[dependencies] +ink = { workspace = true } +scale = { workspace = true } +scale-info = { workspace = true } + +[dev-dependencies] +ink_e2e = { workspace = true } + + diff --git a/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/lib.rs b/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/lib.rs new file mode 100755 index 00000000..d0556766 --- /dev/null +++ b/test-cases/lazy-values-not-set/lazy-values-not-set-1/remediated-example/lib.rs @@ -0,0 +1,148 @@ +#![cfg_attr(not(feature = "std"), no_std, no_main)] + +#[ink::contract] +mod lazy_values_not_set { + use ink::storage::Lazy; + use ink::storage::Mapping; + + #[derive(Debug, PartialEq, Eq, Clone, scale::Encode, scale::Decode)] + #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] + pub enum Error { + /// Caller is not not authorized. + NotAuthorized, + /// Address is invalid. + InvalidAddress, + } + + #[ink(storage)] + pub struct LazyVaulesGetNotSet { + values: Mapping, + lazy_val: Lazy, + } + + impl LazyVaulesGetNotSet { + #[ink(constructor)] + pub fn new() -> Self { + Self { + values: Mapping::default(), + lazy_val: Lazy::default(), + } + } + + #[ink(message)] + pub fn mapping_sum_value_directly(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let val = self.values.get(key).unwrap_or_default(); + self.values.insert(key, &(val + value)); + Ok(()) + } + + #[ink(message)] + pub fn mapping_sum_value_indirectly(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let val = self.values.get(key).unwrap_or_default(); + self.mapping_sum_value_indirectly_step2(val + value) + } + + fn mapping_sum_value_indirectly_step2(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + self.values.insert(key, &value); + Ok(()) + } + + #[ink(message)] + pub fn lazy_sum_value_directly(&mut self, value: u64) -> Result<(), Error> { + let val = self.lazy_val.get().unwrap_or_default(); + self.lazy_val.set(&(val + value)); + Ok(()) + } + + #[ink(message)] + pub fn lazy_sum_value_indirectly(&mut self, value: u64) -> Result<(), Error> { + let val = self.lazy_val.get().unwrap_or_default(); + self.lazy_sum_value_indirectly_step2(val + value) + } + + fn lazy_sum_value_indirectly_step2(&mut self, value: u64) -> Result<(), Error> { + self.lazy_val.set(&value); + Ok(()) + } + + #[ink(message)] + pub fn lazy_sum_value_double_indirectly(&mut self, value: u64) -> Result<(), Error> { + let val = self.lazy_val.get().unwrap_or_default(); + self.lazy_sum_value_double_indirectly_step2(val + value) + } + + fn lazy_sum_value_double_indirectly_step2(&mut self, value: u64) -> Result<(), Error> { + if value > 99999 { + self.lazy_sum_value_double_indirectly_step3(value) + } else { + //test for recursive path + self.lazy_sum_value_double_indirectly(value) + } + } + + fn lazy_sum_value_double_indirectly_step3(&mut self, value: u64) -> Result<(), Error> { + self.lazy_val.set(&value); + Ok(()) + } + } + + impl Default for LazyVaulesGetNotSet { + fn default() -> Self { + Self::new() + } + } + + #[cfg(test)] + mod tests { + use ink::env::test::DefaultAccounts; + + use crate::lazy_values_not_set::LazyVaulesGetNotSet; + + #[ink::test] + fn default_works() { + let contract = LazyVaulesGetNotSet::new(); + assert_eq!(contract.lazy_val.get(), None); + } + + #[ink::test] + fn mapping_sum_value_directly_test() { + let accounts: DefaultAccounts = + ink::env::test::default_accounts::(); + + let mut contract = LazyVaulesGetNotSet::new(); + let res = contract.mapping_sum_value_directly(50); + assert!(res.is_ok()); + assert_eq!(contract.values.get(accounts.alice), Some(50u64)); + } + + #[ink::test] + fn lazy_sum_value_directly_test() { + let mut contract = LazyVaulesGetNotSet::new(); + let res = contract.lazy_sum_value_directly(50); + assert!(res.is_ok()); + assert_eq!(contract.lazy_val.get(), Some(50u64)); + } + + #[ink::test] + fn mapping_sum_value_indirectly_test() { + let accounts: DefaultAccounts = + ink::env::test::default_accounts::(); + + let mut contract = LazyVaulesGetNotSet::new(); + let res = contract.mapping_sum_value_indirectly(50); + assert!(res.is_ok()); + assert_eq!(contract.values.get(accounts.alice), Some(50u64)); + } + + #[ink::test] + fn lazy_sum_value_indirectly_test() { + let mut contract = LazyVaulesGetNotSet::new(); + let res = contract.lazy_sum_value_indirectly(50); + assert!(res.is_ok()); + assert_eq!(contract.lazy_val.get(), Some(50u64)); + } + } +} diff --git a/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/Cargo.toml b/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/Cargo.toml new file mode 100755 index 00000000..44d439f4 --- /dev/null +++ b/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "lazy-values-not-set-vulnerable" +version = "0.1.0" +edition = "2021" +authors = ["[your_name] <[your_email]>"] + +[lib] +path = "lib.rs" + +[features] +default = ["std"] +std = [ + "ink/std", + "scale/std", + "scale-info/std", +] +ink-as-dependency = [] +e2e-tests = [] + +[dependencies] +ink = { workspace = true } +scale = { workspace = true } +scale-info = { workspace = true } + +[dev-dependencies] +ink_e2e = { workspace = true } \ No newline at end of file diff --git a/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/lib.rs b/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/lib.rs new file mode 100755 index 00000000..a915b96c --- /dev/null +++ b/test-cases/lazy-values-not-set/lazy-values-not-set-1/vulnerable-example/lib.rs @@ -0,0 +1,72 @@ +#![cfg_attr(not(feature = "std"), no_std, no_main)] + +#[ink::contract] +mod lazy_values_not_set { + use ink::storage::Lazy; + use ink::storage::Mapping; + + #[derive(Debug, PartialEq, Eq, Clone, scale::Encode, scale::Decode)] + #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] + pub enum Error { + Error, + } + + #[ink(storage)] + pub struct LazyVaulesGetNotSet { + mapping_values_asdf: Mapping, + lazy_val: Lazy, + } + + impl LazyVaulesGetNotSet { + #[ink(constructor)] + pub fn new() -> Self { + Self { + mapping_values_asdf: Mapping::default(), + lazy_val: Lazy::default(), + } + } + + #[ink(message)] + pub fn mapping_sum_value_directly(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let mut _val = self.mapping_values_asdf.get(key).unwrap_or_default(); + _val += value; + Ok(()) + } + + #[ink(message)] + pub fn mapping_sum_value_indirectly(&mut self, value: u64) -> Result<(), Error> { + let key = self.env().caller(); + let val = self.mapping_values_asdf.get(key).unwrap_or_default(); + self.mapping_sum_value_indirectly_step2(val + value) + } + + fn mapping_sum_value_indirectly_step2(&mut self, _value: u64) -> Result<(), Error> { + let _key = self.env().caller(); + Ok(()) + } + + #[ink(message)] + pub fn lazy_sum_value_directly(&mut self, value: u64) -> Result<(), Error> { + let mut _val = self.lazy_val.get().unwrap_or_default(); + _val += value; + Ok(()) + } + + #[ink(message)] + pub fn lazy_sum_value_indirectly(&mut self, value: u64) -> Result<(), Error> { + let val = self.lazy_val.get().unwrap_or_default(); + self.lazy_sum_value_indirectly_step2(val + value) + } + + fn lazy_sum_value_indirectly_step2(&mut self, _value: u64) -> Result<(), Error> { + Ok(()) + } + } + + impl Default for LazyVaulesGetNotSet { + fn default() -> Self { + Self::new() + } + } +} diff --git a/test-cases/vec-could-be-mapping/vec-could-be-mapping-1/remediated-example/src/lib.rs b/test-cases/vec-could-be-mapping/vec-could-be-mapping-1/remediated-example/src/lib.rs index fe18af06..d4b2bddf 100644 --- a/test-cases/vec-could-be-mapping/vec-could-be-mapping-1/remediated-example/src/lib.rs +++ b/test-cases/vec-could-be-mapping/vec-could-be-mapping-1/remediated-example/src/lib.rs @@ -26,7 +26,7 @@ mod vec_could_be_mapping { /// Returns the percentage difference between two values. #[ink(message)] pub fn get_balance(&mut self, acc: AccountId) -> Result { - self.balances.get(&acc).ok_or(Error::NotFound) + self.balances.get(acc).ok_or(Error::NotFound) } } }