unsafe-block test case and detector

.github/workflows/test-detectors.yml

@@ -98,6 +98,7 @@ jobs:
         test:
           [
             "avoid-core-mem-forget",
+            "avoid-unsafe-block",
             "divide-before-multiply",
             "insufficiently-random-values",
             "overflow-check",

detectors/avoid-unsafe-block/Cargo.toml

@@ -0,0 +1,20 @@
+[package]
+name = "avoid-unsafe-block"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+clippy_utils = { workspace = true }
+dylint_linting = { workspace = true }
+if_chain = { workspace = true }
+
+scout-audit-internal = { workspace = true }
+
+[dev-dependencies]
+dylint_testing = { workspace = true }
+
+[package.metadata.rust-analyzer]
+rustc_private = true

detectors/avoid-unsafe-block/src/lib.rs

@@ -0,0 +1,100 @@
+#![feature(rustc_private)]
+
+extern crate rustc_ast;
+extern crate rustc_hir;
+extern crate rustc_span;
+
+use rustc_hir::{
+    intravisit::{walk_expr, Visitor},
+    Expr, ExprKind,
+};
+use rustc_lint::LateLintPass;
+use rustc_span::Span;
+use scout_audit_internal::Detector;
+
+dylint_linting::declare_late_lint! {
+    /// ### What it does
+    /// Checks for usage of `unsafe` blocks.
+    ///
+    /// ### Why is this bad?
+    /// `unsafe` blocks should not be used unless absolutely necessary.
+    ///
+    /// ### Example
+    /// ```rust
+    ///pub fn unsafe_function(n: u64) -> u64 {
+    ///    unsafe {
+    ///        let mut i = n as f64;
+    ///        let mut y = i.to_bits();
+    ///        y = 0x5fe6ec85e7de30da - (y >> 1);
+    ///        i = f64::from_bits(y);
+    ///        i *= 1.5 - 0.5 * n as f64 * i * i;
+    ///        i *= 1.5 - 0.5 * n as f64 * i * i;
+    ///
+    ///        let result_ptr: *mut f64 = &mut i;
+    ///        let result = *result_ptr;
+    ///
+    ///        result.to_bits()
+    ///     }
+    ///}
+    /// Use instead:
+    /// ```rust
+    ///pub fn unsafe_function(n: u64) -> u64 {
+    ///        let mut i = n as f64;
+    ///        let mut y = i.to_bits();
+    ///        y = 0x5fe6ec85e7de30da - (y >> 1);
+    ///        i = f64::from_bits(y);
+    ///        i *= 1.5 - 0.5 * n as f64 * i * i;
+    ///        i *= 1.5 - 0.5 * n as f64 * i * i;
+    ///        result.to_bits()
+    ///}
+    /// ```
+    pub AVOID_UNSAFE_BLOCK,
+    Warn,
+    Detector::AvoidUnsafeBlock.get_lint_message()
+}
+
+impl<'tcx> LateLintPass<'tcx> for AvoidUnsafeBlock {
+    fn check_fn(
+        &mut self,
+        cx: &rustc_lint::LateContext<'tcx>,
+        _: rustc_hir::intravisit::FnKind<'tcx>,
+        _: &'tcx rustc_hir::FnDecl<'tcx>,
+        body: &'tcx rustc_hir::Body<'tcx>,
+        _: rustc_span::Span,
+        _: rustc_hir::def_id::LocalDefId,
+    ) {
+        struct UnsafeBlockVisitor {
+            unsafe_blocks: Vec<Option<Span>>,
+        }
+
+        impl<'tcx> Visitor<'tcx> for UnsafeBlockVisitor {
+            fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+                if let ExprKind::Block(block, _) = expr.kind {
+                    if block.rules
+                        == rustc_hir::BlockCheckMode::UnsafeBlock(
+                            rustc_hir::UnsafeSource::UserProvided,
+                        )
+                    {
+                        self.unsafe_blocks.push(Some(expr.span));
+                    } else {
+                        self.unsafe_blocks.push(None);
+                    }
+                }
+
+                walk_expr(self, expr);
+            }
+        }
+
+        let mut visitor = UnsafeBlockVisitor {
+            unsafe_blocks: Vec::new(),
+        };
+
+        walk_expr(&mut visitor, body.value);
+
+        visitor.unsafe_blocks.iter().for_each(|span| {
+            if let Some(span) = span {
+                Detector::AvoidUnsafeBlock.span_lint(cx, AVOID_UNSAFE_BLOCK, *span);
+            }
+        });
+    }
+}

scout-audit-internal/src/detector.rs

@@ -27,6 +27,7 @@ use strum::{Display, EnumIter};
 #[strum(serialize_all = "kebab-case")]
 pub enum Detector {
     AvoidCoreMemForget,
+    AvoidUnsafeBlock,
     DivideBeforeMultiply,
     InsufficientlyRandomValues,
     OverflowCheck,
@@ -49,6 +50,7 @@ impl Detector {
             Detector::UnprotectedUpdateCurrentContractWasm => {
                 UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE
             }
+            Detector::AvoidUnsafeBlock => AVOID_UNSAFE_BLOCK_LINT_MESSAGE,
             Detector::UnsafeExpect => UNSAFE_EXPECT_LINT_MESSAGE,
             Detector::UnsafeUnwrap => UNSAFE_UNWRAP_LINT_MESSAGE,
         }

scout-audit-internal/src/detector/lint_message.rs

@@ -1,5 +1,7 @@
 pub const AVOID_CORE_MEM_FORGET_LINT_MESSAGE: &str =
     "Use the `let _ = ...` pattern or `.drop()` method to forget the value";
+pub const AVOID_UNSAFE_BLOCK_LINT_MESSAGE: &str =
+    "Avoid using unsafe blocks as it may lead to undefined behavior";
 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";
 

test-cases/avoid-unsafe-block/avoid-unsafe-block-1/remediated-example/Cargo.toml

@@ -0,0 +1,30 @@
+[package]
+name = "avoid-unsafe-block-1-remediated"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+soroban-sdk = { version = "20.0.0-rc2" }
+
+[dev_dependencies]
+soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"] }
+
+[features]
+testutils = ["soroban-sdk/testutils"]
+
+[profile.release]
+opt-level = "z"
+overflow-checks = true
+debug = 0
+strip = "symbols"
+debug-assertions = false
+panic = "abort"
+codegen-units = 1
+lto = true
+
+[profile.release-with-logs]
+inherits = "release"
+debug-assertions = true

test-cases/avoid-unsafe-block/avoid-unsafe-block-1/remediated-example/src/lib.rs

@@ -0,0 +1,33 @@
+#![no_std]
+use soroban_sdk::{contract, contractimpl};
+#[contract]
+pub struct AvoidUnsafeBlock;
+#[contractimpl]
+impl AvoidUnsafeBlock {
+    pub fn unsafe_function(n: u64) -> u64 {
+        let mut i = n as f64;
+        let mut y = i.to_bits();
+        y = 0x5fe6ec85e7de30da - (y >> 1);
+        i = f64::from_bits(y);
+        i *= 1.5 - 0.5 * n as f64 * i * i;
+        i *= 1.5 - 0.5 * n as f64 * i * i;
+        i.to_bits()
+    }
+}
+#[cfg(test)]
+mod tests {
+    use crate::AvoidUnsafeBlock;
+    #[test]
+    fn test_unsafe_block() {
+        let test_value = 8;
+        let result = AvoidUnsafeBlock::unsafe_function(test_value);
+        let inverse = inverse_square_root_without_unsafe(test_value);
+
+        assert_eq!((inverse - result) / inverse, 0);
+        assert_eq!((inverse - result) / result, 0);
+    }
+
+    fn inverse_square_root_without_unsafe(n: u64) -> u64 {
+        (1.0 / (n as f64).sqrt()).to_bits()
+    }
+}

test-cases/avoid-unsafe-block/avoid-unsafe-block-1/vulnerable-example/Cargo.toml

@@ -0,0 +1,35 @@
+[package]
+name = "avoid-unsafe-block-1-vulnerable"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+soroban-sdk = { version = "20.0.0-rc2" }
+
+[dev_dependencies]
+soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"] }
+
+[features]
+testutils = ["soroban-sdk/testutils"]
+
+[profile.release]
+opt-level = "z"
+overflow-checks = true
+debug = 0
+strip = "symbols"
+debug-assertions = false
+panic = "abort"
+codegen-units = 1
+lto = true
+
+[profile.release-with-logs]
+inherits = "release"
+debug-assertions = true
+
+[workspace.metadata.dylint]
+libraries = [
+    { path = "/home/flerena/coinfabrik/soroban/scout-soroban/detectors/unsafe-block"},
+]

test-cases/avoid-unsafe-block/avoid-unsafe-block-1/vulnerable-example/src/lib.rs

@@ -0,0 +1,38 @@
+#![no_std]
+use soroban_sdk::{contract, contractimpl};
+#[contract]
+pub struct AvoidUnsafeBlock;
+#[contractimpl]
+impl AvoidUnsafeBlock {
+    pub fn unsafe_function(n: u64) -> u64 {
+        unsafe {
+            let mut i = n as f64;
+            let mut y = i.to_bits();
+            y = 0x5fe6ec85e7de30da - (y >> 1);
+            i = f64::from_bits(y);
+            i *= 1.5 - 0.5 * n as f64 * i * i;
+            i *= 1.5 - 0.5 * n as f64 * i * i;
+
+            let result_ptr: *mut f64 = &mut i;
+
+            (*result_ptr).to_bits()
+        }
+    }
+}
+#[cfg(test)]
+mod tests {
+    use crate::AvoidUnsafeBlock;
+    #[test]
+    fn test_unsafe_block() {
+        let test_value = 8;
+        let result = AvoidUnsafeBlock::unsafe_function(test_value);
+        let inverse = inverse_square_root_without_unsafe(test_value);
+
+        assert_eq!((inverse - result) / inverse, 0);
+        assert_eq!((inverse - result) / result, 0);
+    }
+
+    fn inverse_square_root_without_unsafe(n: u64) -> u64 {
+        (1.0 / (n as f64).sqrt()).to_bits()
+    }
+}