diff --git a/crates/forge/src/args.rs b/crates/forge/src/args.rs index de7087cecb904..725dcad551286 100644 --- a/crates/forge/src/args.rs +++ b/crates/forge/src/args.rs @@ -130,13 +130,7 @@ pub fn run_command(args: Forge) -> Result<()> { ForgeSubcommand::Flatten(cmd) => cmd.run(), ForgeSubcommand::Inspect(cmd) => cmd.run(), ForgeSubcommand::Tree(cmd) => cmd.run(), - ForgeSubcommand::Geiger(cmd) => { - let n = cmd.run()?; - if n > 0 { - std::process::exit(n as i32); - } - Ok(()) - } + ForgeSubcommand::Geiger(cmd) => cmd.run(), ForgeSubcommand::Doc(cmd) => { if cmd.is_watch() { global.block_on(watch::watch_doc(cmd)) diff --git a/crates/forge/src/cmd/geiger.rs b/crates/forge/src/cmd/geiger.rs index 2caed87583e99..d9390b28d6800 100644 --- a/crates/forge/src/cmd/geiger.rs +++ b/crates/forge/src/cmd/geiger.rs @@ -1,89 +1,38 @@ use clap::{Parser, ValueHint}; -use eyre::{Result, WrapErr}; -use foundry_cli::utils::LoadConfig; -use foundry_compilers::{Graph, resolver::parse::SolData}; -use foundry_config::{Config, impl_figment_convert_basic}; -use itertools::Itertools; -use solar_parse::{ast, ast::visit::Visit, interface::Session}; -use std::{ - ops::ControlFlow, - path::{Path, PathBuf}, -}; +use eyre::Result; +use foundry_cli::opts::BuildOpts; +use foundry_config::impl_figment_convert; +use std::path::PathBuf; /// CLI arguments for `forge geiger`. +/// +/// This command is an alias for `forge lint --only-lint unsafe-cheatcode` +/// and detects usage of unsafe cheat codes in a project and its dependencies. #[derive(Clone, Debug, Parser)] pub struct GeigerArgs { /// Paths to files or directories to detect. #[arg( - conflicts_with = "root", value_hint = ValueHint::FilePath, value_name = "PATH", - num_args(1..), + num_args(0..) )] paths: Vec, - /// The project's root path. - /// - /// By default root of the Git repository, if in one, - /// or the current working directory. - #[arg(long, value_hint = ValueHint::DirPath, value_name = "PATH")] - root: Option, - - /// Globs to ignore. - #[arg( - long, - value_hint = ValueHint::FilePath, - value_name = "PATH", - num_args(1..), - )] - ignore: Vec, - #[arg(long, hide = true)] check: bool, + #[arg(long, hide = true)] full: bool, + + #[command(flatten)] + build: BuildOpts, } -impl_figment_convert_basic!(GeigerArgs); +impl_figment_convert!(GeigerArgs, build); impl GeigerArgs { - pub fn sources(&self, config: &Config) -> Result> { - let cwd = std::env::current_dir()?; - - let mut sources: Vec = { - if self.paths.is_empty() { - let paths = config.project_paths(); - Graph::::resolve(&paths)? - .files() - .keys() - .filter(|f| !paths.has_library_ancestor(f)) - .cloned() - .collect() - } else { - self.paths - .iter() - .flat_map(|path| foundry_common::fs::files_with_ext(path, "sol")) - .unique() - .collect() - } - }; - - sources.retain_mut(|path| { - let abs_path = if path.is_absolute() { path.clone() } else { cwd.join(&path) }; - *path = abs_path.strip_prefix(&cwd).unwrap_or(&abs_path).to_path_buf(); - !self.ignore.iter().any(|ignore| { - if ignore.is_absolute() { - abs_path.starts_with(ignore) - } else { - abs_path.starts_with(cwd.join(ignore)) - } - }) - }); - - Ok(sources) - } - - pub fn run(self) -> Result { + pub fn run(self) -> Result<()> { + // Deprecated flags warnings if self.check { sh_warn!("`--check` is deprecated as it's now the default behavior\n")?; } @@ -91,72 +40,20 @@ impl GeigerArgs { sh_warn!("`--full` is deprecated as reports are not generated anymore\n")?; } - let config = self.load_config()?; - let sources = self.sources(&config).wrap_err("Failed to resolve files")?; - - if config.ffi { - sh_warn!("FFI enabled\n")?; - } - - let mut sess = Session::builder().with_stderr_emitter().build(); - sess.dcx = sess.dcx.set_flags(|flags| flags.track_diagnostics = false); - let unsafe_cheatcodes = &[ - "ffi".to_string(), - "readFile".to_string(), - "readLine".to_string(), - "writeFile".to_string(), - "writeLine".to_string(), - "removeFile".to_string(), - "closeFile".to_string(), - "setEnv".to_string(), - "deriveKey".to_string(), - ]; - Ok(sess - .enter(|| sources.iter().map(|file| lint_file(&sess, unsafe_cheatcodes, file)).sum())) - } -} - -fn lint_file(sess: &Session, unsafe_cheatcodes: &[String], path: &Path) -> usize { - try_lint_file(sess, unsafe_cheatcodes, path).unwrap_or(0) -} - -fn try_lint_file( - sess: &Session, - unsafe_cheatcodes: &[String], - path: &Path, -) -> solar_parse::interface::Result { - let arena = solar_parse::ast::Arena::new(); - let mut parser = solar_parse::Parser::from_file(sess, &arena, path)?; - let ast = parser.parse_file().map_err(|e| e.emit())?; - let mut visitor = Visitor::new(sess, unsafe_cheatcodes); - let _ = visitor.visit_source_unit(&ast); - Ok(visitor.count) -} - -struct Visitor<'a> { - sess: &'a Session, - count: usize, - unsafe_cheatcodes: &'a [String], -} - -impl<'a> Visitor<'a> { - fn new(sess: &'a Session, unsafe_cheatcodes: &'a [String]) -> Self { - Self { sess, count: 0, unsafe_cheatcodes } - } -} - -impl<'ast> Visit<'ast> for Visitor<'_> { - type BreakValue = solar_parse::interface::data_structures::Never; + sh_warn!( + "`forge geiger` is just an alias for `forge lint --only-lint unsafe-cheatcode`\n" + )?; + + // Convert geiger command to lint command with specific lint filter + let lint_args = crate::cmd::lint::LintArgs { + paths: self.paths, + severity: None, + lint: Some(vec!["unsafe-cheatcode".to_string()]), + json: false, + build: self.build, + }; - fn visit_expr(&mut self, expr: &'ast ast::Expr<'ast>) -> ControlFlow { - if let ast::ExprKind::Call(lhs, _args) = &expr.kind - && let ast::ExprKind::Member(_lhs, member) = &lhs.kind - && self.unsafe_cheatcodes.iter().any(|c| c.as_str() == member.as_str()) - { - let msg = format!("usage of unsafe cheatcode `vm.{member}`"); - self.sess.dcx.err(msg).span(member.span).emit(); - self.count += 1; - } - self.walk_expr(expr) + // Run the lint command with the geiger-specific configuration + lint_args.run() } } diff --git a/crates/forge/src/cmd/lint.rs b/crates/forge/src/cmd/lint.rs index 40177370edac9..d4c5cb43f959b 100644 --- a/crates/forge/src/cmd/lint.rs +++ b/crates/forge/src/cmd/lint.rs @@ -17,25 +17,25 @@ use std::path::PathBuf; pub struct LintArgs { /// Path to the file to be checked. Overrides the `ignore` project config. #[arg(value_hint = ValueHint::FilePath, value_name = "PATH", num_args(1..))] - paths: Vec, + pub(crate) paths: Vec, /// Specifies which lints to run based on severity. Overrides the `severity` project config. /// /// Supported values: `high`, `med`, `low`, `info`, `gas`. #[arg(long, value_name = "SEVERITY", num_args(1..))] - severity: Option>, + pub(crate) severity: Option>, /// Specifies which lints to run based on their ID (e.g., "incorrect-shift"). Overrides the /// `exclude_lints` project config. #[arg(long = "only-lint", value_name = "LINT_ID", num_args(1..))] - lint: Option>, + pub(crate) lint: Option>, /// Activates the linter's JSON formatter (rustc-compatible). #[arg(long)] - json: bool, + pub(crate) json: bool, #[command(flatten)] - build: BuildOpts, + pub(crate) build: BuildOpts, } foundry_config::impl_figment_convert!(LintArgs, build); diff --git a/crates/forge/src/opts.rs b/crates/forge/src/opts.rs index 83939781dce94..ae8af21bae05c 100644 --- a/crates/forge/src/opts.rs +++ b/crates/forge/src/opts.rs @@ -145,6 +145,8 @@ pub enum ForgeSubcommand { Tree(tree::TreeArgs), /// Detects usage of unsafe cheat codes in a project and its dependencies. + /// + /// This is an alias for `forge lint --only-lint unsafe-cheatcode`. Geiger(geiger::GeigerArgs), /// Generate documentation for the project. diff --git a/crates/forge/tests/cli/geiger.rs b/crates/forge/tests/cli/geiger.rs index fd21656284744..c224e91f33f31 100644 --- a/crates/forge/tests/cli/geiger.rs +++ b/crates/forge/tests/cli/geiger.rs @@ -1,7 +1,9 @@ -forgetest!(call, |prj, cmd| { - prj.add_source( - "call.sol", +forgetest_init!(call, |prj, cmd| { + prj.add_test( + "call.t.sol", r#" + import {Test} from "forge-std/Test.sol"; + contract A is Test { function do_ffi() public { string[] memory inputs = new string[](1); @@ -12,22 +14,25 @@ forgetest!(call, |prj, cmd| { ) .unwrap(); - cmd.arg("geiger").assert_code(1).stderr_eq(str![[r#" -error: usage of unsafe cheatcode `vm.ffi` - [FILE]:7:20 + cmd.arg("geiger").assert_success().stderr_eq(str![[r#" +... +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + [FILE]:9:20 | -7 | vm.ffi(inputs); - | ^^^ +9 | vm.ffi(inputs); + | --- | - - + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode +... "#]]); }); -forgetest!(assignment, |prj, cmd| { - prj.add_source( - "assignment.sol", +forgetest_init!(assignment, |prj, cmd| { + prj.add_test( + "assignment.t.sol", r#" + import {Test} from "forge-std/Test.sol"; + contract A is Test { function do_ffi() public { string[] memory inputs = new string[](1); @@ -38,24 +43,28 @@ forgetest!(assignment, |prj, cmd| { ) .unwrap(); - cmd.arg("geiger").assert_code(1).stderr_eq(str![[r#" -error: usage of unsafe cheatcode `vm.ffi` - [FILE]:7:34 + cmd.arg("geiger").assert_success().stderr_eq(str![[r#" +... +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + [FILE]:9:34 | -7 | bytes stuff = vm.ffi(inputs); - | ^^^ +9 | bytes stuff = vm.ffi(inputs); + | --- | - - + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode +... "#]]); }); -forgetest!(exit_code, |prj, cmd| { - prj.add_source( - "multiple.sol", +forgetest_init!(exit_code, |prj, cmd| { + prj.add_test( + "multiple.t.sol", r#" + import {Test} from "forge-std/Test.sol"; + contract A is Test { function do_ffi() public { + string[] memory inputs = new string[](1); vm.ffi(inputs); vm.ffi(inputs); vm.ffi(inputs); @@ -65,28 +74,31 @@ forgetest!(exit_code, |prj, cmd| { ) .unwrap(); - cmd.arg("geiger").assert_code(3).stderr_eq(str![[r#" -error: usage of unsafe cheatcode `vm.ffi` - [FILE]:6:20 - | -6 | vm.ffi(inputs); - | ^^^ - | - -error: usage of unsafe cheatcode `vm.ffi` - [FILE]:7:20 - | -7 | vm.ffi(inputs); - | ^^^ - | - -error: usage of unsafe cheatcode `vm.ffi` - [FILE]:8:20 + cmd.arg("geiger").assert_success().stderr_eq(str![[r#" +... +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + [FILE]:9:20 | -8 | vm.ffi(inputs); - | ^^^ +9 | vm.ffi(inputs); + | --- | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + [FILE]:10:20 + | +10 | vm.ffi(inputs); + | --- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + [FILE]:11:20 + | +11 | vm.ffi(inputs); + | --- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode +... "#]]); }); diff --git a/crates/lint/src/sol/info/mod.rs b/crates/lint/src/sol/info/mod.rs index 50f6122325427..75b6c433f6777 100644 --- a/crates/lint/src/sol/info/mod.rs +++ b/crates/lint/src/sol/info/mod.rs @@ -12,10 +12,14 @@ use screaming_snake_case::{SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_I mod imports; use imports::{UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT}; +mod unsafe_cheatcodes; +use unsafe_cheatcodes::UNSAFE_CHEATCODE_USAGE; + register_lints!( (PascalCaseStruct, early, (PASCAL_CASE_STRUCT)), (MixedCaseVariable, early, (MIXED_CASE_VARIABLE)), (MixedCaseFunction, early, (MIXED_CASE_FUNCTION)), (ScreamingSnakeCase, early, (SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE)), - (Imports, early, (UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT)) + (Imports, early, (UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT)), + (UnsafeCheatcodes, early, (UNSAFE_CHEATCODE_USAGE)) ); diff --git a/crates/lint/src/sol/info/unsafe_cheatcodes.rs b/crates/lint/src/sol/info/unsafe_cheatcodes.rs new file mode 100644 index 0000000000000..3b48129b82a9a --- /dev/null +++ b/crates/lint/src/sol/info/unsafe_cheatcodes.rs @@ -0,0 +1,36 @@ +use super::UnsafeCheatcodes; +use crate::{ + linter::{EarlyLintPass, LintContext}, + sol::{Severity, SolLint}, +}; +use solar_ast::{Expr, ExprKind}; + +declare_forge_lint!( + UNSAFE_CHEATCODE_USAGE, + Severity::Info, + "unsafe-cheatcode", + "usage of unsafe cheatcodes that can perform dangerous operations" +); + +const UNSAFE_CHEATCODES: [&str; 9] = [ + "ffi", + "readFile", + "readLine", + "writeFile", + "writeLine", + "removeFile", + "closeFile", + "setEnv", + "deriveKey", +]; + +impl<'ast> EarlyLintPass<'ast> for UnsafeCheatcodes { + fn check_expr(&mut self, ctx: &LintContext<'_>, expr: &'ast Expr<'ast>) { + if let ExprKind::Call(lhs, _args) = &expr.kind + && let ExprKind::Member(_lhs, member) = &lhs.kind + && UNSAFE_CHEATCODES.iter().any(|&c| c == member.as_str()) + { + ctx.emit(&UNSAFE_CHEATCODE_USAGE, member.span); + } + } +} diff --git a/crates/lint/testdata/UnsafeCheatcodes.sol b/crates/lint/testdata/UnsafeCheatcodes.sol new file mode 100644 index 0000000000000..122d0176935de --- /dev/null +++ b/crates/lint/testdata/UnsafeCheatcodes.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {Test} from "./auxiliary/Test.sol"; + +contract UnsafeCheatcodes is Test { + function testSafeCheatcodes() public { + vm.prank(address(0x1)); + vm.deal(address(0x1), 1 ether); + vm.warp(block.timestamp + 1); + vm.roll(block.number + 1); + vm.assume(true); + vm.expectRevert(); + } + + function testDirectFfi() public { + string[] memory inputs = new string[](1); + vm.ffi(inputs); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectReadFile() public { + vm.readFile("test.txt"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectReadLine() public { + vm.readLine("test.txt"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectWriteFile() public { + vm.writeFile("test.txt", "data"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectWriteLine() public { + vm.writeLine("test.txt", "data"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectRemoveFile() public { + vm.removeFile("test.txt"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectCloseFile() public { + vm.closeFile("test.txt"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectSetEnv() public { + vm.setEnv("KEY", "value"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testDirectDeriveKey() public { + vm.deriveKey("mnemonic", 0); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testAssignmentFfi() public { + string[] memory inputs = new string[](1); + bytes memory result = vm.ffi(inputs); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + + function testMultipleUnsafe() public { + vm.ffi(new string[](1)); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + vm.setEnv("KEY", "value"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + vm.readFile("test.txt"); //~NOTE: usage of unsafe cheatcodes that can perform dangerous operations + } + +} diff --git a/crates/lint/testdata/UnsafeCheatcodes.stderr b/crates/lint/testdata/UnsafeCheatcodes.stderr new file mode 100644 index 0000000000000..0be680f4501ba --- /dev/null +++ b/crates/lint/testdata/UnsafeCheatcodes.stderr @@ -0,0 +1,104 @@ +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +18 | vm.ffi(inputs); + | --- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +22 | vm.readFile("test.txt"); + | -------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +26 | vm.readLine("test.txt"); + | -------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +30 | vm.writeFile("test.txt", "data"); + | --------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +34 | vm.writeLine("test.txt", "data"); + | --------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +38 | vm.removeFile("test.txt"); + | ---------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +42 | vm.closeFile("test.txt"); + | --------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +46 | vm.setEnv("KEY", "value"); + | ------ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +50 | vm.deriveKey("mnemonic", 0); + | --------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +55 | bytes memory result = vm.ffi(inputs); + | --- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +59 | vm.ffi(new string[](1)); + | --- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +60 | vm.setEnv("KEY", "value"); + | ------ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + +note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations + --> ROOT/testdata/UnsafeCheatcodes.sol:LL:CC + | +61 | vm.readFile("test.txt"); + | -------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode + diff --git a/crates/lint/testdata/auxiliary/Test.sol b/crates/lint/testdata/auxiliary/Test.sol new file mode 100644 index 0000000000000..a4e518950e5fc --- /dev/null +++ b/crates/lint/testdata/auxiliary/Test.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +contract Test { + Vm vm; +} + +interface Vm { + // Unsafe cheatcodes + function ffi(string[] calldata) external returns (bytes memory); + function readFile(string calldata) external returns (string memory); + function readLine(string calldata) external returns (string memory); + function writeFile(string calldata, string calldata) external; + function writeLine(string calldata, string calldata) external; + function removeFile(string calldata) external; + function closeFile(string calldata) external; + function setEnv(string calldata, string calldata) external; + function deriveKey(string calldata, uint32) external returns (uint256); + + // Safe cheatcodes + function prank(address) external; + function deal(address, uint256) external; + function warp(uint256) external; + function roll(uint256) external; + function assume(bool) external; + function expectRevert() external; +}