From c0c3d6c67637fcfdc64aa704ed3cc614a98d11b7 Mon Sep 17 00:00:00 2001 From: Facundo Lerena Date: Mon, 11 Dec 2023 13:30:21 -0300 Subject: [PATCH] changed core-mem-forget to avoid-.. (#50) Changed name from core-mem-forget to avoid-core-mem-forget for readability --- .github/workflows/test-detectors.yml | 2 +- .../Cargo.toml | 2 +- .../src/lib.rs | 49 ++++++++++++------- scout-audit-internal/src/detector.rs | 7 +-- .../src/detector/lint_message.rs | 6 ++- .../remediated-example}/Cargo.toml | 2 +- .../remediated-example/src/lib.rs | 9 ++-- .../vulnerable-example}/Cargo.toml | 2 +- .../vulnerable-example/src/lib.rs | 9 ++-- 9 files changed, 48 insertions(+), 40 deletions(-) rename detectors/{core-mem-forget => avoid-core-mem-forget}/Cargo.toml (91%) rename detectors/{core-mem-forget => avoid-core-mem-forget}/src/lib.rs (69%) rename test-cases/{core-mem-forget/core-mem-forget-1/vulnerable-example => avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example}/Cargo.toml (91%) rename test-cases/{core-mem-forget/core-mem-forget-1 => avoid-core-mem-forget/avoid-core-mem-forget-1}/remediated-example/src/lib.rs (75%) rename test-cases/{core-mem-forget/core-mem-forget-1/remediated-example => avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example}/Cargo.toml (91%) rename test-cases/{core-mem-forget/core-mem-forget-1 => avoid-core-mem-forget/avoid-core-mem-forget-1}/vulnerable-example/src/lib.rs (75%) diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index ad876ad6..95deb93c 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -97,7 +97,7 @@ jobs: - macos-latest test: [ - "core-mem-forget", + "avoid-core-mem-forget", "divide-before-multiply", "insufficiently-random-values", "overflow-check", diff --git a/detectors/core-mem-forget/Cargo.toml b/detectors/avoid-core-mem-forget/Cargo.toml similarity index 91% rename from detectors/core-mem-forget/Cargo.toml rename to detectors/avoid-core-mem-forget/Cargo.toml index fc07fb1e..ac154ea0 100644 --- a/detectors/core-mem-forget/Cargo.toml +++ b/detectors/avoid-core-mem-forget/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "core-mem-forget" +name = "avoid-core-mem-forget" version = "0.1.0" edition = "2021" diff --git a/detectors/core-mem-forget/src/lib.rs b/detectors/avoid-core-mem-forget/src/lib.rs similarity index 69% rename from detectors/core-mem-forget/src/lib.rs rename to detectors/avoid-core-mem-forget/src/lib.rs index 8d2f9c30..fa421c5e 100644 --- a/detectors/core-mem-forget/src/lib.rs +++ b/detectors/avoid-core-mem-forget/src/lib.rs @@ -17,32 +17,42 @@ dylint_linting::impl_pre_expansion_lint! { /// This is a bad practice because it can lead to memory leaks, resource leaks and logic errors. /// ### Example /// ```rust - /// pub fn forget_something(n: WithoutCopy) -> u64 { - /// core::mem::forget(n); - /// 0 - /// } - /// ``` + /// pub fn forget_value(&mut self) { + /// let forgotten_value = self.value; + /// self.value = false; + /// core::mem::forget(forgotten_value); + /// } /// + /// ``` /// Use instead: - /// ```rust - /// pub fn forget_something(n: WithoutCopy) -> u64 { - /// let _ = n; - /// 0 - /// } - /// ``` + ///```rust + /// pub fn forget_value(&mut self) { + /// let forgotten_value = self.value; + /// self.value = false; + /// let _ = forgotten_value; + /// } + /// + /// // or use drop if droppable + /// + /// pub fn drop_value(&mut self) { + /// let forgotten_value = self.value; + /// self.value = false; + /// forget_value.drop(); + /// } + ///``` - pub CORE_MEM_FORGET, + pub AVOID_CORE_MEM_FORGET, Warn, - Detector::CoreMemForget.get_lint_message(), - CoreMemForget::default() + Detector::AvoidCoreMemForget.get_lint_message(), + AvoidCoreMemForget::default() } #[derive(Default)] -pub struct CoreMemForget { +pub struct AvoidCoreMemForget { stack: Vec, } -impl EarlyLintPass for CoreMemForget { +impl EarlyLintPass for AvoidCoreMemForget { fn check_item(&mut self, _cx: &EarlyContext, item: &Item) { if self.in_test_item() || is_test_item(item) { self.stack.push(item.id); @@ -59,9 +69,10 @@ impl EarlyLintPass for CoreMemForget { if path.segments[1].ident.name.to_string() == "mem"; if path.segments[2].ident.name.to_string() == "forget"; then { - Detector::CoreMemForget.span_lint_and_help( + + Detector::AvoidCoreMemForget.span_lint_and_help( cx, - CORE_MEM_FORGET, + AVOID_CORE_MEM_FORGET, expr.span, "Instead, use the `let _ = ...` pattern or `.drop` method to forget the value.", ); @@ -91,7 +102,7 @@ fn is_test_item(item: &Item) -> bool { }) } -impl CoreMemForget { +impl AvoidCoreMemForget { fn in_test_item(&self) -> bool { !self.stack.is_empty() } diff --git a/scout-audit-internal/src/detector.rs b/scout-audit-internal/src/detector.rs index 81908728..9ba0cd89 100644 --- a/scout-audit-internal/src/detector.rs +++ b/scout-audit-internal/src/detector.rs @@ -26,7 +26,7 @@ use strum::{Display, EnumIter}; #[derive(Debug, Display, Clone, EnumIter, PartialEq, Eq, Hash)] #[strum(serialize_all = "kebab-case")] pub enum Detector { - CoreMemForget, + AvoidCoreMemForget, DivideBeforeMultiply, InsufficientlyRandomValues, OverflowCheck, @@ -40,9 +40,10 @@ impl Detector { /// Returns the lint message for the detector. pub const fn get_lint_message(&self) -> &'static str { match self { - Detector::CoreMemForget => CORE_MEM_FORGET_LINT_MESSAGE, - Detector::DivideBeforeMultiply => DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE, + Detector::AvoidCoreMemForget => AVOID_CORE_MEM_FORGET_LINT_MESSAGE, Detector::InsufficientlyRandomValues => INSUFFICIENTLY_RANDOM_VALUES_LINT_MESSAGE, + + Detector::DivideBeforeMultiply => DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE, Detector::OverflowCheck => OVERFLOW_CHECK_LINT_MESSAGE, Detector::SetContractStorage => SET_CONTRACT_STORAGE_LINT_MESSAGE, Detector::UnprotectedUpdateCurrentContractWasm => { diff --git a/scout-audit-internal/src/detector/lint_message.rs b/scout-audit-internal/src/detector/lint_message.rs index 4663ebaf..23df1253 100644 --- a/scout-audit-internal/src/detector/lint_message.rs +++ b/scout-audit-internal/src/detector/lint_message.rs @@ -1,8 +1,10 @@ -pub const CORE_MEM_FORGET_LINT_MESSAGE: &str = +pub const AVOID_CORE_MEM_FORGET_LINT_MESSAGE: &str = "Use the `let _ = ...` pattern or `.drop()` method to forget the value"; +pub const INSUFFICIENTLY_RANDOM_VALUES_LINT_MESSAGE: &str = + "Use env.prng() to generate random numbers, and remember that all random numbers are under the control of validators"; + pub const DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE: &str = "Division before multiplication might result in a loss of precision"; -pub const INSUFFICIENTLY_RANDOM_VALUES_LINT_MESSAGE: &str = "Use env.prng() to generate random numbers, and remember that all random numbers are under the control of validators"; pub const OVERFLOW_CHECK_LINT_MESSAGE: &str = "Use `overflow-checks = true` in Cargo.toml profile"; pub const SET_CONTRACT_STORAGE_LINT_MESSAGE:&str = "Abitrary users should not have control over keys because it implies writing any value of left mapping, lazy variable, or the main struct of the contract located in position 0 of the storage"; pub const UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE: &str = diff --git a/test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/Cargo.toml b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/Cargo.toml similarity index 91% rename from test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/Cargo.toml rename to test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/Cargo.toml index 9b848f19..1b643e7c 100644 --- a/test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/Cargo.toml +++ b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "core-mem-forget-1-vulnerable" +name = "avoid-core-mem-forget-1-remediated" version = "0.1.0" edition = "2021" diff --git a/test-cases/core-mem-forget/core-mem-forget-1/remediated-example/src/lib.rs b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/src/lib.rs similarity index 75% rename from test-cases/core-mem-forget/core-mem-forget-1/remediated-example/src/lib.rs rename to test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/src/lib.rs index 0e6fab6b..e6789bb7 100644 --- a/test-cases/core-mem-forget/core-mem-forget-1/remediated-example/src/lib.rs +++ b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/remediated-example/src/lib.rs @@ -2,7 +2,7 @@ use soroban_sdk::{contract, contractimpl, contracttype}; #[contract] -pub struct CoreMemForget; +pub struct AvoidCoreMemForget; #[contracttype] #[derive(Eq, PartialEq)] @@ -12,7 +12,7 @@ pub struct WithoutCopy { } #[contractimpl] -impl CoreMemForget { +impl AvoidCoreMemForget { pub fn forget_something(n: WithoutCopy) -> u64 { let _ = n; 0 @@ -25,13 +25,10 @@ mod tests { #[test] fn test_forget_something() { - // Given let test_value: WithoutCopy = WithoutCopy { a: 80, b: 60 }; - // When - let result = CoreMemForget::forget_something(test_value); + let result = AvoidCoreMemForget::forget_something(test_value); - // Then assert_eq!(result, 0); } } diff --git a/test-cases/core-mem-forget/core-mem-forget-1/remediated-example/Cargo.toml b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/Cargo.toml similarity index 91% rename from test-cases/core-mem-forget/core-mem-forget-1/remediated-example/Cargo.toml rename to test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/Cargo.toml index 9d6db30e..863f16e0 100644 --- a/test-cases/core-mem-forget/core-mem-forget-1/remediated-example/Cargo.toml +++ b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "core-mem-forget-1-remediated" +name = "avoid-core-mem-forget-1-vulnerable" version = "0.1.0" edition = "2021" diff --git a/test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/src/lib.rs b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/src/lib.rs similarity index 75% rename from test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/src/lib.rs rename to test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/src/lib.rs index 6d993d95..9f8aa0df 100644 --- a/test-cases/core-mem-forget/core-mem-forget-1/vulnerable-example/src/lib.rs +++ b/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1/vulnerable-example/src/lib.rs @@ -2,7 +2,7 @@ use soroban_sdk::{contract, contractimpl, contracttype}; #[contract] -pub struct CoreMemForget; +pub struct AvoidCoreMemForget; #[contracttype] #[derive(Eq, PartialEq)] @@ -12,7 +12,7 @@ pub struct WithoutCopy { } #[contractimpl] -impl CoreMemForget { +impl AvoidCoreMemForget { pub fn forget_something(n: WithoutCopy) -> u64 { core::mem::forget(n); 0 @@ -25,13 +25,10 @@ mod tests { #[test] fn test_forget_something() { - // Given let test_value: WithoutCopy = WithoutCopy { a: 80, b: 60 }; - // When - let result = CoreMemForget::forget_something(test_value); + let result = AvoidCoreMemForget::forget_something(test_value); - // Then assert_eq!(result, 0); } }