Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changed core-mem-forget to avoid-.. #50

Merged
merged 11 commits into from
Dec 11, 2023
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 @@ -39,9 +39,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::UnprotectedUpdateCurrentContractWasm => {
UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE
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 UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE: &str =
"This update_current_contract_wasm is called without access control";
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);
}
}