diff --git a/detectors/front-running/Cargo.toml b/detectors/front-running/Cargo.toml new file mode 100644 index 00000000..48cb9894 --- /dev/null +++ b/detectors/front-running/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2021" +name = "front-running" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +clippy_utils = { workspace = true } +clippy_wrappers = { workspace = true } +dylint_linting = { workspace = true } +if_chain = { workspace = true } +utils = { workspace = true } + +[package.metadata.rust-analyzer] +rustc_private = true diff --git a/detectors/front-running/src/conditional_checker.rs b/detectors/front-running/src/conditional_checker.rs new file mode 100644 index 00000000..b6a56ab7 --- /dev/null +++ b/detectors/front-running/src/conditional_checker.rs @@ -0,0 +1,64 @@ +use rustc_hir::{def::Res, BinOpKind, Expr, ExprKind, HirId, QPath, UnOp}; + +const PANIC_INDUCING_FUNCTIONS: [&str; 2] = ["panic", "bail"]; + +#[derive(Clone, Copy, Hash, Eq, PartialEq, Default, Debug)] +pub struct ConditionalChecker { + pub greater_expr: Option, + pub lesser_expr: Option, +} + +impl ConditionalChecker { + fn handle_binary_op(&mut self, op: BinOpKind, left: &Expr, right: &Expr) -> bool { + match op { + BinOpKind::Ge | BinOpKind::Gt => { + self.greater_expr = get_res_hir_id(left); + self.lesser_expr = get_res_hir_id(right); + true + } + BinOpKind::Le | BinOpKind::Lt => { + self.lesser_expr = get_res_hir_id(left); + self.greater_expr = get_res_hir_id(right); + true + } + _ => false, + } + } + + pub fn handle_condition(&mut self, condition: &Expr) -> bool { + match &condition.kind { + ExprKind::Binary(op, left, right) => self.handle_binary_op(op.node, left, right), + ExprKind::Unary(UnOp::Not, unary_expr) => { + if let ExprKind::Binary(op, left, right) = &unary_expr.kind { + self.handle_binary_op(op.node, left, right) + } else { + false + } + } + _ => false, + } + } +} + +pub fn get_res_hir_id(expr: &Expr) -> Option { + if let ExprKind::Path(QPath::Resolved(_, path)) = &expr.kind { + if let Res::Local(hir_id) = path.res { + Some(hir_id) + } else { + None + } + } else { + None + } +} + +pub fn is_panic_inducing_call(func: &Expr<'_>) -> bool { + if let ExprKind::Path(QPath::Resolved(_, path)) = &func.kind { + return PANIC_INDUCING_FUNCTIONS.iter().any(|&func| { + path.segments + .iter() + .any(|segment| segment.ident.name.as_str().contains(func)) + }); + } + false +} diff --git a/detectors/front-running/src/lib.rs b/detectors/front-running/src/lib.rs new file mode 100644 index 00000000..20a6d267 --- /dev/null +++ b/detectors/front-running/src/lib.rs @@ -0,0 +1,195 @@ +#![feature(rustc_private)] + +extern crate rustc_hir; +extern crate rustc_span; + +mod conditional_checker; + +use clippy_utils::higher::If; +use clippy_wrappers::span_lint; +use conditional_checker::{get_res_hir_id, is_panic_inducing_call, ConditionalChecker}; +use if_chain::if_chain; +use rustc_hir::{ + def::Res::{self}, + intravisit::{walk_expr, FnKind, Visitor}, + Body, Expr, ExprKind, FnDecl, HirId, LetStmt, Path, QPath, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::{ + def_id::{DefId, LocalDefId}, + Span, Symbol, +}; +use std::{ + collections::{HashMap, HashSet}, + vec, +}; +use utils::{get_node_type_opt, FunctionCallVisitor}; + +const LINT_MESSAGE: &str = + "The transferred amount should be checked against a minimum to prevent front-running"; + +dylint_linting::impl_late_lint! { + pub FRONT_RUNNING, + Warn, + LINT_MESSAGE, + FrontRunning::default(), + { + name: "Front Running Detection", + long_message: "This lint checks for potential front-running vulnerabilities in token transfers", + severity: "Warning", + help: "Consider implementing a minimum amount check before the transfer", + vulnerability_class: "MEV", + } +} + +#[derive(Default)] +struct FrontRunning { + function_call_graph: HashMap>, + checked_functions: HashSet, +} + +impl<'tcx> LateLintPass<'tcx> for FrontRunning { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + local_def_id: LocalDefId, + ) { + let def_id = local_def_id.to_def_id(); + self.checked_functions.insert(cx.tcx.def_path_str(def_id)); + + if span.from_expansion() { + return; + } + + let mut function_call_visitor = + FunctionCallVisitor::new(cx, def_id, &mut self.function_call_graph); + function_call_visitor.visit_body(body); + + let function_params: HashSet<_> = + body.params.iter().map(|param| param.pat.hir_id).collect(); + + let mut front_running_visitor = FrontRunningVisitor { + checked_hir_ids: HashSet::new(), + conditional_checker: Vec::new(), + cx, + filtered_local_variables: HashSet::new(), + function_params, + transfer_amount_id: Vec::new(), + }; + front_running_visitor.visit_body(body); + + for (transfer_amount_id, span) in &front_running_visitor.transfer_amount_id { + if !front_running_visitor + .checked_hir_ids + .contains(transfer_amount_id) + && front_running_visitor + .filtered_local_variables + .contains(transfer_amount_id) + { + span_lint(cx, FRONT_RUNNING, *span, LINT_MESSAGE); + } + } + } +} + +struct FrontRunningVisitor<'a, 'tcx> { + checked_hir_ids: HashSet, + conditional_checker: Vec, + cx: &'a LateContext<'tcx>, + filtered_local_variables: HashSet, + function_params: HashSet, + transfer_amount_id: Vec<(HirId, Span)>, +} + +impl FrontRunningVisitor<'_, '_> { + fn add_to_checked_hir_ids(&mut self, last_conditional_checker: ConditionalChecker) { + if let (Some(lesser_hir_id), Some(greater_hir_id)) = ( + last_conditional_checker.lesser_expr, + last_conditional_checker.greater_expr, + ) { + if self.function_params.contains(&lesser_hir_id) { + self.checked_hir_ids.insert(greater_hir_id); + } + } + } + + fn local_uses_parameter(&self, expr: &Expr) -> bool { + match &expr.kind { + ExprKind::MethodCall(_, _, args, _) | ExprKind::Call(_, args) => { + args.iter().any(|arg| { + get_res_hir_id(arg) + .map_or(false, |hir_id| self.function_params.contains(&hir_id)) + }) + } + ExprKind::Binary(_, left_expr, right_expr) => { + self.local_uses_parameter(left_expr) || self.local_uses_parameter(right_expr) + } + _ => false, + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for FrontRunningVisitor<'a, 'tcx> { + fn visit_local(&mut self, local: &'tcx LetStmt<'tcx>) { + if let Some(init) = &local.init { + if self.local_uses_parameter(init) { + self.filtered_local_variables.insert(local.pat.hir_id); + } + } + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + // Check if the expression is a transfer method call, then store the HirId of the amount parameter + if_chain! { + if let ExprKind::MethodCall(path_segment, receiver, args, ..) = expr.kind; + if path_segment.ident.name == Symbol::intern("transfer"); + if let Some(receiver_type) = get_node_type_opt(self.cx, &receiver.hir_id); + if receiver_type.to_string() == "soroban_sdk::token::TokenClient<'_>"; + if let ExprKind::AddrOf(_, _, amount_expr, ..) = args[2].kind; + if let ExprKind::Path(QPath::Resolved(_, Path { segments, .. }), ..) = amount_expr.kind; + if let Some(segment) = segments.first(); + if let Res::Local(hir_id) = segment.res; + then { + self.transfer_amount_id.push((hir_id, expr.span)); + } + } + + // If we are inside an 'if' statement, check if the current expression is a return or a panic inducing call + if let Some(last_conditional_checker) = self.conditional_checker.last().copied() { + match &expr.kind { + ExprKind::Ret(..) => { + self.add_to_checked_hir_ids(last_conditional_checker); + } + ExprKind::Call(func, _) if is_panic_inducing_call(func) => { + self.add_to_checked_hir_ids(last_conditional_checker); + } + _ => {} + } + } + + // Check if the expression has an 'if' and if it does, check if it meets our condition + if let Some(If { + cond, + then: if_expr, + r#else: _, + }) = If::hir(expr) + { + let mut conditional_checker = ConditionalChecker { + greater_expr: None, + lesser_expr: None, + }; + if conditional_checker.handle_condition(cond) { + self.conditional_checker.push(conditional_checker); + walk_expr(self, if_expr); + self.conditional_checker.pop(); + return; + } + } + + walk_expr(self, expr); + } +} diff --git a/test-cases/front-running/Cargo.toml b/test-cases/front-running/Cargo.toml new file mode 100644 index 00000000..5a1bfab3 --- /dev/null +++ b/test-cases/front-running/Cargo.toml @@ -0,0 +1,22 @@ +[workspace] +exclude = [".cargo", "target"] +members = ["front-running-*/*"] +resolver = "2" + +[workspace.dependencies] +soroban-sdk = { version = "=20.0.0" } + +[profile.release] +codegen-units = 1 +debug = 0 +debug-assertions = false +lto = true +opt-level = "z" +overflow-checks = true +panic = "abort" +strip = "symbols" + +[profile.release-with-logs] +debug-assertions = true +inherits = "release" + diff --git a/test-cases/front-running/front-running-1/remediated-example/Cargo.toml b/test-cases/front-running/front-running-1/remediated-example/Cargo.toml new file mode 100644 index 00000000..a9947e6b --- /dev/null +++ b/test-cases/front-running/front-running-1/remediated-example/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2021" +name = "front-running-remediated-1" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { version = "20.0.0" } +soroban-token-sdk = { version = "20.0.0" } + +[dev-dependencies] +soroban-sdk = { version = "20.0.0", features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/front-running/front-running-1/remediated-example/src/lib.rs b/test-cases/front-running/front-running-1/remediated-example/src/lib.rs new file mode 100644 index 00000000..6795865a --- /dev/null +++ b/test-cases/front-running/front-running-1/remediated-example/src/lib.rs @@ -0,0 +1,173 @@ +#![no_std] + +use soroban_sdk::{ + contract, contractimpl, contracttype, + token::{StellarAssetClient, TokenClient}, + Address, Env, +}; + +#[contracttype] +pub enum DataKey { + Token, +} + +#[contract] +pub struct FrontRunning; + +#[contractimpl] +impl FrontRunning { + pub fn init(e: Env, contract: Address) { + e.storage().persistent().set(&DataKey::Token, &contract); + } + + pub fn get_token(e: Env) -> Address { + get_token(&e) + } + + pub fn mint(e: Env, to: Address, amount: i128) { + StellarAssetClient::new(&e, &get_token(&e)).mint(&to, &amount); + } + + pub fn approve(e: Env, from: Address, spender: Address, amount: i128, expiration_ledger: u32) { + TokenClient::new(&e, &get_token(&e)).approve(&from, &spender, &amount, &expiration_ledger); + } + + pub fn transfer(e: Env, from: Address, to: Address, amount: i128, min_amount: i128) { + let transfer_amount = get_conversion_price(amount); + assert!(transfer_amount >= min_amount, "Insufficient amount"); + TokenClient::new(&e, &get_token(&e)).transfer(&from, &to, &transfer_amount); + } + + pub fn allowance(e: Env, from: Address, spender: Address) -> i128 { + TokenClient::new(&e, &get_token(&e)).allowance(&from, &spender) + } +} + +fn get_token(e: &Env) -> Address { + e.storage().persistent().get(&DataKey::Token).unwrap() +} + +fn get_conversion_price(amount: i128) -> i128 { + // This function symbolizes a change in the state of the blockchain + 100 * amount +} + +#[cfg(test)] +mod tests { + extern crate std; + + use soroban_sdk::{ + testutils::{AuthorizedFunction, AuthorizedInvocation}, + token::TokenClient, + Address, Env, IntoVal, Symbol, + }; + + use crate::{FrontRunning, FrontRunningClient}; + use soroban_sdk::testutils::Address as _; + + #[test] + fn test_approve() { + // Given + let env = Env::default(); + let admin = Address::generate(&env); + let token_contract_id = env.register_stellar_asset_contract(admin); + + let contract_id = env.register_contract(None, FrontRunning); + let client = FrontRunningClient::new(&env, &contract_id); + client.init(&token_contract_id); + + // When + let from = Address::generate(&env); + let spender = Address::generate(&env); + + // Then + client + .mock_all_auths_allowing_non_root_auth() + .approve(&from, &spender, &200, &200); + + let auths = env.auths(); + assert_eq!( + auths, + std::vec![( + from.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + token_contract_id.clone(), + Symbol::new(&env, "approve"), + (&from, &spender, 200_i128, 200_u32).into_val(&env) + )), + sub_invocations: std::vec![] + } + )] + ); + + // Check allowance + let token_client = TokenClient::new(&env, &client.get_token()); + assert_eq!(token_client.allowance(&from, &spender), 200); + } + + #[test] + fn test_transfer() { + // Given + let env = Env::default(); + let admin = Address::generate(&env); + let token_contract_id = env.register_stellar_asset_contract(admin); + + let contract_id = env.register_contract(None, FrontRunning); + let client = FrontRunningClient::new(&env, &contract_id); + client.init(&token_contract_id); + + // When + let token_client = TokenClient::new(&env, &client.get_token()); + assert_eq!(token_client.decimals(), 7); + let from = Address::generate(&env); + let to = Address::generate(&env); + + // Mint tokens to the `from` address + client + .mock_all_auths_allowing_non_root_auth() + .mint(&from, &10000); + + // Then + client + .mock_all_auths_allowing_non_root_auth() + .transfer(&from, &to, &1, &0); + + // Check final balances + let from_balance = token_client.balance(&from); + let to_balance = token_client.balance(&to); + + assert_eq!(from_balance, 10000 - 100); + assert_eq!(to_balance, 100); + } + + #[test] + #[should_panic(expected = "Insufficient amount")] + fn test_front_running() { + // Given + let env = Env::default(); + let admin = Address::generate(&env); + let token_contract_id = env.register_stellar_asset_contract(admin); + + let contract_id = env.register_contract(None, FrontRunning); + let client = FrontRunningClient::new(&env, &contract_id); + client.init(&token_contract_id); + + // When + let token_client = TokenClient::new(&env, &client.get_token()); + assert_eq!(token_client.decimals(), 7); + let from = Address::generate(&env); + let to = Address::generate(&env); + + // Mint tokens to the `from` address + client + .mock_all_auths_allowing_non_root_auth() + .mint(&from, &10000); + + // Then + client + .mock_all_auths_allowing_non_root_auth() + .transfer(&from, &to, &100, &10001); + // The contract should now panic since the transfer amount is less than the minimum amount + } +} diff --git a/test-cases/front-running/front-running-1/vulnerable-example/Cargo.toml b/test-cases/front-running/front-running-1/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..77555903 --- /dev/null +++ b/test-cases/front-running/front-running-1/vulnerable-example/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2021" +name = "front-running-vulnerable-1" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { version = "20.0.0" } +soroban-token-sdk = { version = "20.0.0" } + +[dev-dependencies] +soroban-sdk = { version = "20.0.0", features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/front-running/front-running-1/vulnerable-example/src/lib.rs b/test-cases/front-running/front-running-1/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..b7fbf131 --- /dev/null +++ b/test-cases/front-running/front-running-1/vulnerable-example/src/lib.rs @@ -0,0 +1,142 @@ +#![no_std] + +use soroban_sdk::{ + contract, contractimpl, contracttype, + token::{StellarAssetClient, TokenClient}, + Address, Env, +}; + +#[contracttype] +pub enum DataKey { + Token, +} + +#[contract] +pub struct FrontRunning; + +#[contractimpl] +impl FrontRunning { + pub fn init(e: Env, contract: Address) { + e.storage().persistent().set(&DataKey::Token, &contract); + } + + pub fn get_token(e: Env) -> Address { + get_token(&e) + } + + pub fn mint(e: Env, to: Address, amount: i128) { + StellarAssetClient::new(&e, &get_token(&e)).mint(&to, &amount); + } + + pub fn approve(e: Env, from: Address, spender: Address, amount: i128, expiration_ledger: u32) { + TokenClient::new(&e, &get_token(&e)).approve(&from, &spender, &amount, &expiration_ledger); + } + + pub fn transfer(e: Env, from: Address, to: Address, amount: i128) { + let transfer_amount = get_conversion_price(amount); + TokenClient::new(&e, &get_token(&e)).transfer(&from, &to, &transfer_amount); + } + + pub fn allowance(e: Env, from: Address, spender: Address) -> i128 { + TokenClient::new(&e, &get_token(&e)).allowance(&from, &spender) + } +} + +fn get_token(e: &Env) -> Address { + e.storage().persistent().get(&DataKey::Token).unwrap() +} + +fn get_conversion_price(amount: i128) -> i128 { + // This function symbolizes a change in the state of the blockchain + 100 * amount +} + +#[cfg(test)] +mod tests { + extern crate std; + + use soroban_sdk::{ + testutils::{AuthorizedFunction, AuthorizedInvocation}, + token::TokenClient, + Address, Env, IntoVal, Symbol, + }; + + use crate::{FrontRunning, FrontRunningClient}; + use soroban_sdk::testutils::Address as _; + + #[test] + fn test_approve() { + // Given + let env = Env::default(); + let admin = Address::generate(&env); + let token_contract_id = env.register_stellar_asset_contract(admin); + + let contract_id = env.register_contract(None, FrontRunning); + let client = FrontRunningClient::new(&env, &contract_id); + client.init(&token_contract_id); + + // When + let from = Address::generate(&env); + let spender = Address::generate(&env); + + // Then + client + .mock_all_auths_allowing_non_root_auth() + .approve(&from, &spender, &200, &200); + + let auths = env.auths(); + assert_eq!( + auths, + std::vec![( + from.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + token_contract_id.clone(), + Symbol::new(&env, "approve"), + (&from, &spender, 200_i128, 200_u32).into_val(&env) + )), + sub_invocations: std::vec![] + } + )] + ); + + // Check allowance + let token_client = TokenClient::new(&env, &client.get_token()); + assert_eq!(token_client.allowance(&from, &spender), 200); + } + + #[test] + fn test_transfer() { + // Given + let env = Env::default(); + let admin = Address::generate(&env); + let token_contract_id = env.register_stellar_asset_contract(admin); + + let contract_id = env.register_contract(None, FrontRunning); + let client = FrontRunningClient::new(&env, &contract_id); + client.init(&token_contract_id); + + // When + let token_client = TokenClient::new(&env, &client.get_token()); + assert_eq!(token_client.decimals(), 7); + let from = Address::generate(&env); + let to = Address::generate(&env); + + // Mint tokens to the `from` address + client + .mock_all_auths_allowing_non_root_auth() + .mint(&from, &10000); + + // Then + client + .mock_all_auths_allowing_non_root_auth() + .transfer(&from, &to, &1); + + // Check final balances (optional) + let from_balance = token_client.balance(&from); + let to_balance = token_client.balance(&to); + + assert_eq!(from_balance, 10000 - 100); + assert_eq!(to_balance, 100); + } +}