diff --git a/detectors/Cargo.toml b/detectors/Cargo.toml index 2eafc924..a03df777 100644 --- a/detectors/Cargo.toml +++ b/detectors/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -exclude = [".cargo", "target", ".vscode"] +exclude = [".cargo", ".vscode", "target"] members = ["*"] resolver = "2" diff --git a/detectors/divide-before-multiply/src/lib.rs b/detectors/divide-before-multiply/src/lib.rs index 00d293a7..70a8f14f 100644 --- a/detectors/divide-before-multiply/src/lib.rs +++ b/detectors/divide-before-multiply/src/lib.rs @@ -177,8 +177,9 @@ fn navigate_trough_basicblocks<'tcx>( if BinOp::Div == *op { tainted_places.push(assign.0); } else if BinOp::Mul == *op - && (check_operand(&operands.0, tainted_places, &assign.0) - || check_operand(&operands.1, tainted_places, &assign.0)) + || BinOp::MulWithOverflow == *op + && (check_operand(&operands.0, tainted_places, &assign.0) + || check_operand(&operands.1, tainted_places, &assign.0)) { spans.push(statement.source_info.span); }; diff --git a/detectors/integer-overflow-or-underflow/Cargo.toml b/detectors/integer-overflow-or-underflow/Cargo.toml new file mode 100644 index 00000000..15f1a7fd --- /dev/null +++ b/detectors/integer-overflow-or-underflow/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +clippy_utils = { workspace = true } +dylint_linting = { workspace = true } +if_chain = { workspace = true } +utils = { workspace = true } + +[package.metadata.rust-analyzer] +rustc_private = true diff --git a/detectors/integer-overflow-or-underflow/src/lib.rs b/detectors/integer-overflow-or-underflow/src/lib.rs new file mode 100644 index 00000000..031282ca --- /dev/null +++ b/detectors/integer-overflow-or-underflow/src/lib.rs @@ -0,0 +1,224 @@ +#![feature(rustc_private)] + +extern crate rustc_hir; +extern crate rustc_span; + +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_hir::{ + intravisit::{walk_expr, FnKind, Visitor}, + BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::{def_id::LocalDefId, Span, Symbol}; +use std::collections::HashSet; +use utils::ConstantAnalyzer; + +pub const LINT_MESSAGE: &str = "Potential for integer arithmetic overflow/underflow. Consider checked, wrapping or saturating arithmetic."; + +dylint_linting::declare_late_lint! { + pub INTEGER_OVERFLOW_UNDERFLOW, + Warn, + LINT_MESSAGE, + { + name: "Integer Overflow/Underflow", + long_message: "An overflow/underflow is typically caught and generates an error. When it is not caught, the operation will result in an inexact result which could lead to serious problems.", + severity: "Critical", + help: "https://coinfabrik.github.io/scout-soroban/docs/vulnerabilities/integer-overflow-or-underflow", + vulnerability_class: "Arithmetic", + } +} +enum Type { + Overflow, + Underflow, + OverflowUnderflow, +} + +impl Type { + fn message(&self) -> &'static str { + match self { + Type::Overflow => "overflow", + Type::Underflow => "underflow", + Type::OverflowUnderflow => "overflow or underflow", + } + } +} + +enum Cause { + Add, + Sub, + Mul, + Pow, + Negate, + Multiple, +} + +impl Cause { + fn message(&self) -> &'static str { + match self { + Cause::Add => "addition operation", + Cause::Sub => "subtraction operation", + Cause::Mul => "multiplication operation", + Cause::Pow => "exponentiation operation", + Cause::Negate => "negation operation", + Cause::Multiple => "operation", + } + } +} + +pub struct Finding { + span: Span, + type_: Type, + cause: Cause, +} + +impl Finding { + fn new(span: Span, type_: Type, cause: Cause) -> Self { + Finding { span, type_, cause } + } + + fn generate_message(&self) -> String { + format!( + "This {} could {}.", + self.cause.message(), + self.type_.message() + ) + } +} +pub struct IntegerOverflowUnderflowVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + findings: Vec, + is_complex_operation: bool, + constant_analyzer: ConstantAnalyzer<'a, 'tcx>, +} + +impl<'tcx> IntegerOverflowUnderflowVisitor<'_, 'tcx> { + pub fn check_pow(&mut self, expr: &Expr<'tcx>, base: &Expr<'tcx>, exponent: &Expr<'tcx>) { + if self.constant_analyzer.is_constant(base) && self.constant_analyzer.is_constant(exponent) + { + return; + } + + let base_type = self.cx.typeck_results().expr_ty(base); + if base_type.is_integral() { + self.findings + .push(Finding::new(expr.span, Type::Overflow, Cause::Pow)); + } + } + + pub fn check_negate(&mut self, expr: &Expr<'tcx>, operand: &Expr<'tcx>) { + if self.constant_analyzer.is_constant(operand) { + return; + } + + let operand_type = self.cx.typeck_results().expr_ty(operand); + if operand_type.is_integral() && operand_type.is_signed() { + self.findings + .push(Finding::new(expr.span, Type::Overflow, Cause::Negate)); + } + } + + pub fn check_binary( + &mut self, + expr: &Expr<'tcx>, + op: BinOpKind, + left: &Expr<'tcx>, + right: &Expr<'tcx>, + ) { + if self.constant_analyzer.is_constant(left) && self.constant_analyzer.is_constant(right) { + return; + } + + let (left_type, right_type) = ( + self.cx.typeck_results().expr_ty(left).peel_refs(), + self.cx.typeck_results().expr_ty(right).peel_refs(), + ); + if !left_type.is_integral() || !right_type.is_integral() { + return; + } + + let (finding_type, cause) = if self.is_complex_operation { + (Type::OverflowUnderflow, Cause::Multiple) + } else { + match op { + BinOpKind::Add => (Type::Overflow, Cause::Add), + BinOpKind::Sub => (Type::Underflow, Cause::Sub), + BinOpKind::Mul => (Type::Overflow, Cause::Mul), + _ => return, + } + }; + + self.findings + .push(Finding::new(expr.span, finding_type, cause)); + } +} + +impl<'a, 'tcx> Visitor<'tcx> for IntegerOverflowUnderflowVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + match expr.kind { + ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => { + self.is_complex_operation = matches!(lhs.kind, ExprKind::Binary(..)) + || matches!(rhs.kind, ExprKind::Binary(..)); + self.check_binary(expr, op.node, lhs, rhs); + if self.is_complex_operation { + return; + } + } + ExprKind::Unary(UnOp::Neg, arg) => { + self.check_negate(expr, arg); + } + ExprKind::MethodCall(method_name, receiver, args, ..) => { + if method_name.ident.name == Symbol::intern("pow") { + self.check_pow(expr, receiver, &args[0]); + } + } + _ => (), + } + + walk_expr(self, expr); + } +} + +impl<'tcx> LateLintPass<'tcx> for IntegerOverflowUnderflow { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + _: LocalDefId, + ) { + // If the function comes from a macro expansion, we ignore it + if span.from_expansion() { + return; + } + + // Gather all compile-time variables in the function + let mut constant_analyzer = ConstantAnalyzer { + cx, + constants: HashSet::new(), + }; + constant_analyzer.visit_body(body); + + // Analyze the function for integer overflow/underflow + let mut visitor = IntegerOverflowUnderflowVisitor { + cx, + findings: Vec::new(), + is_complex_operation: false, + constant_analyzer, + }; + visitor.visit_body(body); + + // Report any findings + for finding in visitor.findings { + span_lint_and_help( + cx, + INTEGER_OVERFLOW_UNDERFLOW, + finding.span, + finding.generate_message(), + None, + "Consider using the checked version of this operation/s", + ) + } + } +} diff --git a/test-cases/integer-overflow-or-underflow/Cargo.toml b/test-cases/integer-overflow-or-underflow/Cargo.toml new file mode 100644 index 00000000..ca2a186c --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/Cargo.toml @@ -0,0 +1,21 @@ +[workspace] +exclude = [".cargo", "target"] +members = ["integer-overflow-or-underflow-*/*"] +resolver = "2" + +[workspace.dependencies] +soroban-sdk = { version = "=21.3.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/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/Cargo.toml new file mode 100644 index 00000000..0c8cdb76 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-remediated-1" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/src/lib.rs new file mode 100644 index 00000000..a5d17cdf --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/remediated-example/src/lib.rs @@ -0,0 +1,56 @@ +#![no_std] +use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + OverflowError = 1, +} + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn add(env: Env, value: u32) -> Result<(), Error> { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = match current.checked_add(value) { + Some(value) => value, + None => return Err(Error::OverflowError), + }; + env.storage().temporary().set(&Self::VALUE, &new_value); + Ok(()) + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + fn test_add_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&u32::MAX); + let result = client.try_add(&1); + + // Then + assert_eq!(result, Err(Ok(Error::OverflowError))); + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..d143c46a --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-vulnerable-1" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..41b6fe4d --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1/vulnerable-example/src/lib.rs @@ -0,0 +1,46 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn add(env: Env, value: u32) { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = current + value; + env.storage().temporary().set(&Self::VALUE, &new_value); + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + #[should_panic(expected = "attempt to add with overflow")] + fn test_add_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&u32::MAX); + client.add(&1); + + // Then + // Panic + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/Cargo.toml new file mode 100644 index 00000000..abbb5ad6 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-remediated-2" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/src/lib.rs new file mode 100644 index 00000000..df001569 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/remediated-example/src/lib.rs @@ -0,0 +1,56 @@ +#![no_std] +use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + UnderflowError = 1, +} + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn sub(env: Env, value: u32) -> Result<(), Error> { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = match current.checked_sub(value) { + Some(value) => value, + None => return Err(Error::UnderflowError), + }; + env.storage().temporary().set(&Self::VALUE, &new_value); + Ok(()) + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + fn test_sub_underflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&0); + let result = client.try_sub(&1); + + // Then + assert_eq!(result, Err(Ok(Error::UnderflowError))); + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..d1571812 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-vulnerable-2" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..83a57ea9 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2/vulnerable-example/src/lib.rs @@ -0,0 +1,46 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn sub(env: Env, value: u32) { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = current - value; + env.storage().temporary().set(&Self::VALUE, &new_value); + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + #[should_panic(expected = "attempt to subtract with overflow")] + fn test_sub_underflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&0); + client.sub(&1); + + // Then + // Panic + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/Cargo.toml new file mode 100644 index 00000000..0f8c6824 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-remediated-3" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/src/lib.rs new file mode 100644 index 00000000..c51cee28 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/remediated-example/src/lib.rs @@ -0,0 +1,57 @@ +#![no_std] +use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + OverflowError = 1, + UnderflowError = 2, +} + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn mul(env: Env, value: u32) -> Result<(), Error> { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = match current.checked_mul(value) { + Some(value) => value, + None => return Err(Error::OverflowError), + }; + env.storage().temporary().set(&Self::VALUE, &new_value); + Ok(()) + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + fn test_mul_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&u32::MAX); + let result = client.try_mul(&2); + + // Then + assert_eq!(result, Err(Ok(Error::OverflowError))); + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..cbd5cf7d --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-vulnerable-3" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] + diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..7564b34b --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-3/vulnerable-example/src/lib.rs @@ -0,0 +1,47 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn mul(env: Env, value: u32) { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = current * value; + env.storage().temporary().set(&Self::VALUE, &new_value); + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + #[should_panic(expected = "attempt to multiply with overflow")] + fn test_mul_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&u32::MAX); + client.mul(&2); + + // Then + // Panic + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/Cargo.toml new file mode 100644 index 00000000..4c1bd994 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-remediated-4" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/src/lib.rs new file mode 100644 index 00000000..53272651 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/remediated-example/src/lib.rs @@ -0,0 +1,57 @@ +#![no_std] +use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + OverflowError = 1, + UnderflowError = 2, +} + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn pow(env: Env, value: u32) -> Result<(), Error> { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = match current.checked_pow(value) { + Some(value) => value, + None => return Err(Error::OverflowError), + }; + env.storage().temporary().set(&Self::VALUE, &new_value); + Ok(()) + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + fn test_pow_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&2); + let result = client.try_pow(&u32::MAX); + + // Then + assert_eq!(result, Err(Ok(Error::OverflowError))); + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..2dc0ba18 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-vulnerable-4" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] + diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..34619e1a --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-4/vulnerable-example/src/lib.rs @@ -0,0 +1,47 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: u32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn pow(env: Env, value: u32) { + let current: u32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = current.pow(value); + env.storage().temporary().set(&Self::VALUE, &new_value); + } + + pub fn get(env: Env) -> u32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + #[should_panic(expected = "attempt to multiply with overflow")] + fn test_pow_overflow() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&2); + client.pow(&u32::MAX); + + // Then + // Panic + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/Cargo.toml new file mode 100644 index 00000000..d2fc3df9 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-remediated-5" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/src/lib.rs new file mode 100644 index 00000000..691cf47b --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/remediated-example/src/lib.rs @@ -0,0 +1,57 @@ +#![no_std] +use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + OverflowError = 1, + UnderflowError = 2, +} + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: i32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn neg(env: Env) -> Result<(), Error> { + let current: i32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = match current.checked_neg() { + Some(value) => value, + None => return Err(Error::UnderflowError), + }; + env.storage().temporary().set(&Self::VALUE, &new_value); + Ok(()) + } + + pub fn get(env: Env) -> i32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + fn test_neg() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&i32::MIN); + let result = client.try_neg(); + + // Then + assert_eq!(result, Err(Ok(Error::UnderflowError))); + } +} diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/Cargo.toml b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..75390e80 --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "integer-overflow-or-underflow-vulnerable-5" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/src/lib.rs b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..b9d1e04c --- /dev/null +++ b/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-5/vulnerable-example/src/lib.rs @@ -0,0 +1,47 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; + +#[contract] +pub struct IntegerOverflowUnderflow; + +#[contractimpl] +impl IntegerOverflowUnderflow { + const VALUE: Symbol = symbol_short!("VALUE"); + + pub fn initialize(env: Env, value: i32) { + env.storage().temporary().set(&Self::VALUE, &value); + } + + pub fn neg(env: Env) { + let current: i32 = env.storage().temporary().get(&Self::VALUE).unwrap_or(0); + let new_value = -current; + env.storage().temporary().set(&Self::VALUE, &new_value); + } + + pub fn get(env: Env) -> i32 { + env.storage().temporary().get(&Self::VALUE).unwrap_or(0) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::Env; + + #[test] + #[should_panic(expected = "attempt to negate with overflow")] + fn test_neg() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, IntegerOverflowUnderflow); + let client = IntegerOverflowUnderflowClient::new(&env, &contract_id); + + // When + client.initialize(&i32::MIN); + client.neg(); + + // Then + // Panic + } +} diff --git a/utils/Cargo.lock b/utils/Cargo.lock index 7cda8d78..7004de71 100644 --- a/utils/Cargo.lock +++ b/utils/Cargo.lock @@ -2,15 +2,222 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "arrayvec" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" + +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + +[[package]] +name = "clippy_config" +version = "0.1.81" +source = "git+https://github.com/rust-lang/rust-clippy?rev=51a1cf0#51a1cf07876d36a5008f9dc07cd36ba9d412a5b7" +dependencies = [ + "rustc-semver", + "serde", + "toml", +] + +[[package]] +name = "clippy_utils" +version = "0.1.81" +source = "git+https://github.com/rust-lang/rust-clippy?rev=51a1cf0#51a1cf07876d36a5008f9dc07cd36ba9d412a5b7" +dependencies = [ + "arrayvec", + "clippy_config", + "itertools", + "rustc-semver", + "rustc_apfloat", +] + +[[package]] +name = "either" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" + +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "if_chain" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb56e1aa765b4b4f3aadfab769793b7087bb03a4ea4920644a6d238e2df5b9ed" +[[package]] +name = "indexmap" +version = "2.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" +dependencies = [ + "equivalent", + "hashbrown", +] + +[[package]] +name = "itertools" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" +dependencies = [ + "either", +] + +[[package]] +name = "memchr" +version = "2.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" + +[[package]] +name = "proc-macro2" +version = "1.0.86" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e719e8df665df0d1c8fbfd238015744736151d4445ec0836b8e628aae103b77" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustc-semver" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5be1bdc7edf596692617627bbfeaba522131b18e06ca4df2b6b689e3c5d5ce84" + +[[package]] +name = "rustc_apfloat" +version = "0.2.1+llvm-462a31f5a5ab" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "886d94c63c812a8037c4faca2607453a0fa4cf82f734665266876b022244543f" +dependencies = [ + "bitflags", + "smallvec", +] + +[[package]] +name = "serde" +version = "1.0.204" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc76f558e0cbb2a839d37354c575f1dc3fdc6546b5be373ba43d95f231bf7c12" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.204" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0cd7e117be63d3c3678776753929474f3b04a43a080c744d6b0ae2a8c28e222" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_spanned" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb5b1b31579f3811bf615c144393417496f152e12ac8b7663bf664f4a815306d" +dependencies = [ + "serde", +] + +[[package]] +name = "smallvec" +version = "1.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" + +[[package]] +name = "syn" +version = "2.0.72" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc4b9b9bf2add8093d3f2c0204471e951b2285580335de42f9d2534f3ae7a8af" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "toml" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd79e69d3b627db300ff956027cc6c3798cef26d22526befdfcd12feeb6d2257" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8fb9f64314842840f1d940ac544da178732128f1c78c21772e876579e0da1db" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.19.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b5bb770da30e5cbfde35a2d7b9b8a2c4b8ef89548a7a6aeab5c9a576e3e7421" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "winnow", +] + +[[package]] +name = "unicode-ident" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" + [[package]] name = "utils" version = "0.1.0" dependencies = [ + "clippy_utils", "if_chain", ] + +[[package]] +name = "winnow" +version = "0.5.40" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f593a95398737aeed53e489c785df13f3618e41dbcd6718c6addbf1395aa6876" +dependencies = [ + "memchr", +] diff --git a/utils/Cargo.toml b/utils/Cargo.toml index 5bf141dc..f4cd3b28 100644 --- a/utils/Cargo.toml +++ b/utils/Cargo.toml @@ -4,7 +4,8 @@ name = "utils" version = "0.1.0" [dependencies] -if_chain = "1.0.2" +if_chain = "=1.0.2" +clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "51a1cf0" } [package.metadata.rust-analyzer] rustc_private = true diff --git a/utils/src/constant_analyzer/mod.rs b/utils/src/constant_analyzer/mod.rs new file mode 100644 index 00000000..a9e9457e --- /dev/null +++ b/utils/src/constant_analyzer/mod.rs @@ -0,0 +1,115 @@ +extern crate rustc_hir; +extern crate rustc_lint; + +use clippy_utils::consts::{constant, Constant}; +use if_chain::if_chain; +use rustc_hir::{ + def::{DefKind, Res}, + intravisit::{walk_local, Visitor}, + Expr, ExprKind, HirId, Node, QPath, +}; +use rustc_lint::LateContext; +use std::collections::HashSet; + +/// Analyzes expressions to determine if they are constants or known at compile-time. +pub struct ConstantAnalyzer<'a, 'tcx> { + pub cx: &'a LateContext<'tcx>, + pub constants: HashSet, +} + +impl<'a, 'tcx> ConstantAnalyzer<'a, 'tcx> { + /// Checks if a QPath refers to a constant. + fn is_qpath_constant(&self, path: &QPath) -> bool { + if let QPath::Resolved(_, path) = path { + match path.res { + Res::Def(def_kind, _) => matches!( + def_kind, + DefKind::AnonConst + | DefKind::AssocConst + | DefKind::Const + | DefKind::InlineConst + ), + Res::Local(hir_id) => self.constants.contains(&hir_id), + _ => false, + } + } else { + false + } + } + + /// Determines if an expression is constant or known at compile-time. + fn is_expr_constant(&self, expr: &Expr<'tcx>) -> bool { + if constant(self.cx, self.cx.typeck_results(), expr).is_some() { + return true; + } + + match expr.kind { + ExprKind::Array(expr_array) => expr_array + .iter() + .all(|expr_in_array| self.is_expr_constant(expr_in_array)), + ExprKind::Binary(_, left_expr, right_expr) => { + self.is_expr_constant(left_expr) && self.is_expr_constant(right_expr) + } + ExprKind::Cast(cast_expr, _) => self.is_expr_constant(cast_expr), + ExprKind::Field(field_expr, _) => self.is_expr_constant(field_expr), + ExprKind::Index(array_expr, index_expr, _) => { + self.is_array_index_constant(array_expr, index_expr) + } + ExprKind::Lit(_) => true, + ExprKind::Path(qpath_expr) => self.is_qpath_constant(&qpath_expr), + ExprKind::Repeat(repeat_expr, _) => self.is_expr_constant(repeat_expr), + ExprKind::Struct(_, expr_fields, _) => expr_fields + .iter() + .all(|field_expr| self.is_expr_constant(field_expr.expr)), + _ => false, + } + } + + /// Checks if an array index operation results in a constant value. + fn is_array_index_constant(&self, array_expr: &Expr<'tcx>, index_expr: &Expr<'tcx>) -> bool { + match ( + &array_expr.kind, + constant(self.cx, self.cx.typeck_results(), index_expr), + ) { + (ExprKind::Array(array_elements), Some(Constant::Int(index))) => { + self.is_array_element_constant(array_elements, index) + } + (ExprKind::Path(QPath::Resolved(_, path)), Some(Constant::Int(index))) => { + if_chain! { + if let Res::Local(hir_id) = path.res; + if let Node::LetStmt(let_stmt) = self.cx.tcx.parent_hir_node(hir_id); + if let Some(ExprKind::Array(array_elements)) = let_stmt.init.map(|init| &init.kind); + then { + self.is_array_element_constant(array_elements, index) + } else { + false + } + } + } + _ => false, + } + } + + /// Checks if a specific array element is constant. + fn is_array_element_constant(&self, elements: &[Expr<'tcx>], index: u128) -> bool { + elements + .get(index as usize) + .map_or(false, |element| self.is_expr_constant(element)) + } + + /// Public method to check if an expression is constant. + pub fn is_constant(&self, expr: &Expr<'tcx>) -> bool { + self.is_expr_constant(expr) + } +} + +impl<'a, 'tcx> Visitor<'tcx> for ConstantAnalyzer<'a, 'tcx> { + fn visit_local(&mut self, l: &'tcx rustc_hir::LetStmt<'tcx>) -> Self::Result { + if let Some(init) = l.init { + if self.is_expr_constant(init) { + self.constants.insert(l.pat.hir_id); + } + } + walk_local(self, l); + } +} diff --git a/utils/src/lib.rs b/utils/src/lib.rs index 352bef89..22c331d8 100644 --- a/utils/src/lib.rs +++ b/utils/src/lib.rs @@ -8,3 +8,6 @@ pub use soroban_utils::*; mod lint_utils; pub use lint_utils::*; + +mod constant_analyzer; +pub use constant_analyzer::*;