Skip to content

Commit

Permalink
changed core-mem-forget to avoid-.. (#50)
Browse files Browse the repository at this point in the history
Changed name from core-mem-forget to avoid-core-mem-forget for readability
  • Loading branch information
faculerena authored Dec 11, 2023
1 parent f4a56b9 commit c0c3d6c
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 40 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-detectors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ jobs:
- macos-latest
test:
[
"core-mem-forget",
"avoid-core-mem-forget",
"divide-before-multiply",
"insufficiently-random-values",
"overflow-check",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "core-mem-forget"
name = "avoid-core-mem-forget"
version = "0.1.0"
edition = "2021"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<NodeId>,
}

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);
Expand All @@ -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.",
);
Expand Down Expand Up @@ -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()
}
Expand Down
7 changes: 4 additions & 3 deletions scout-audit-internal/src/detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 => {
Expand Down
6 changes: 4 additions & 2 deletions scout-audit-internal/src/detector/lint_message.rs
Original file line number Diff line number Diff line change
@@ -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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "core-mem-forget-1-vulnerable"
name = "avoid-core-mem-forget-1-remediated"
version = "0.1.0"
edition = "2021"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use soroban_sdk::{contract, contractimpl, contracttype};

#[contract]
pub struct CoreMemForget;
pub struct AvoidCoreMemForget;

#[contracttype]
#[derive(Eq, PartialEq)]
Expand All @@ -12,7 +12,7 @@ pub struct WithoutCopy {
}

#[contractimpl]
impl CoreMemForget {
impl AvoidCoreMemForget {
pub fn forget_something(n: WithoutCopy) -> u64 {
let _ = n;
0
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "core-mem-forget-1-remediated"
name = "avoid-core-mem-forget-1-vulnerable"
version = "0.1.0"
edition = "2021"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use soroban_sdk::{contract, contractimpl, contracttype};

#[contract]
pub struct CoreMemForget;
pub struct AvoidCoreMemForget;

#[contracttype]
#[derive(Eq, PartialEq)]
Expand All @@ -12,7 +12,7 @@ pub struct WithoutCopy {
}

#[contractimpl]
impl CoreMemForget {
impl AvoidCoreMemForget {
pub fn forget_something(n: WithoutCopy) -> u64 {
core::mem::forget(n);
0
Expand All @@ -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);
}
}

0 comments on commit c0c3d6c

Please sign in to comment.