Skip to content

Commit b21d972

Browse files
stevencartaviazerosnacks
andauthored
feat(lint): add shared analysis helpers for Solidity lints (foundry-rs#14706)
* feat(lint): add shared analysis helpers for Solidity lints * refactor(lint): trim analysis helpers to used set and migrate consumers * preserve original receiver_contract_id matching * comments --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
1 parent 054857e commit b21d972

9 files changed

Lines changed: 182 additions & 122 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//! Match contract function shapes (ABI signatures, receiver contract type).
2+
3+
use solar::sema::hir::{self, ContractId, Expr, ExprKind, ItemId, Res, TypeKind, VariableId};
4+
5+
/// True if `id`'s elementary type matches the given ABI string.
6+
pub fn is_elementary(hir: &hir::Hir<'_>, id: VariableId, abi: &str) -> bool {
7+
matches!(&hir.variable(id).ty.kind, TypeKind::Elementary(ty) if ty.to_abi_str() == abi)
8+
}
9+
10+
/// Static contract type of a method-call receiver: a contract-typed variable
11+
/// or an `IFoo(addr)` interface wrap.
12+
pub fn receiver_contract_id(hir: &hir::Hir<'_>, recv: &Expr<'_>) -> Option<ContractId> {
13+
match &recv.kind {
14+
ExprKind::Ident([Res::Item(ItemId::Variable(id)), ..]) => {
15+
if let TypeKind::Custom(ItemId::Contract(cid)) = hir.variable(*id).ty.kind {
16+
Some(cid)
17+
} else {
18+
None
19+
}
20+
}
21+
ExprKind::Call(
22+
Expr { kind: ExprKind::Ident([Res::Item(ItemId::Contract(cid))]), .. },
23+
..,
24+
) => Some(*cid),
25+
_ => None,
26+
}
27+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//! Shared analysis primitives reused by Solidity lints.
2+
//!
3+
//! - [`primitives`]: HIR probes (`is_address_type`, `is_require_or_assert`,
4+
//! `address_call_receiver`, `branch_always_exits`).
5+
//! - [`interface`]: contract/library function-shape matching (`is_elementary`,
6+
//! `receiver_contract_id`).
7+
//!
8+
//! All helpers borrow HIR and never mutate it.
9+
10+
pub mod interface;
11+
pub mod primitives;
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
//! Side-effect-free syntactic/semantic probes over solar HIR.
2+
3+
use solar::{
4+
ast::LitKind,
5+
interface::{Symbol, kw, sym},
6+
sema::hir::{self, ElementaryType, Expr, ExprKind, Res, Stmt, StmtKind, TypeKind, VariableId},
7+
};
8+
9+
/// True if `expr` references the named global builtin (`msg`, `tx`, `this`, ...).
10+
fn is_builtin(expr: &Expr<'_>, name: Symbol) -> bool {
11+
matches!(&expr.peel_parens().kind, ExprKind::Ident(reses)
12+
if reses.iter().any(|r| matches!(r, Res::Builtin(b) if b.name() == name)))
13+
}
14+
15+
/// True if `vid` is typed as `address`/`address payable`.
16+
pub fn is_address_type(hir: &hir::Hir<'_>, vid: VariableId) -> bool {
17+
matches!(hir.variable(vid).ty.kind, TypeKind::Elementary(ElementaryType::Address(_)))
18+
}
19+
20+
/// True if `callee` resolves to the builtin `require` or `assert`.
21+
pub fn is_require_or_assert(callee: &Expr<'_>) -> bool {
22+
matches!(&callee.kind, ExprKind::Ident(reses)
23+
if reses.iter().any(|r| matches!(r,
24+
Res::Builtin(b) if b.name() == sym::require || b.name() == sym::assert)))
25+
}
26+
27+
/// Receiver of `<expr>.{call,delegatecall,transfer,send}`, including the
28+
/// `.call{value: x}(...)` option form.
29+
pub fn address_call_receiver<'a>(callee: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
30+
// `addr.call{...}(..)` lowers as `Call(Member(receiver, "call"), ..)`.
31+
let inner = match &callee.kind {
32+
ExprKind::Call(inner, ..) => inner,
33+
_ => callee,
34+
};
35+
let target = if matches!(inner.kind, ExprKind::Member(..)) { inner } else { callee };
36+
if let ExprKind::Member(receiver, name) = &target.kind {
37+
let n = name.name;
38+
if n == kw::Call || n == kw::Delegatecall || n == sym::transfer || n == sym::send {
39+
return Some(receiver);
40+
}
41+
}
42+
None
43+
}
44+
45+
/// True when executing `stmt` provably prevents control from continuing past
46+
/// it: a `return`, `revert`/`revert(...)`, `require(false, ...)`,
47+
/// `assert(false)`, a block containing any such statement (any subsequent
48+
/// statements are unreachable), or an `if` whose both arms exit.
49+
pub fn branch_always_exits(stmt: &Stmt<'_>) -> bool {
50+
match &stmt.kind {
51+
StmtKind::Return(_) | StmtKind::Revert(_) => true,
52+
StmtKind::Expr(expr) => is_exit_call(expr),
53+
StmtKind::Block(b) | StmtKind::UncheckedBlock(b) => b.stmts.iter().any(branch_always_exits),
54+
StmtKind::If(_, t, Some(e)) => branch_always_exits(t) && branch_always_exits(e),
55+
_ => false,
56+
}
57+
}
58+
59+
fn is_exit_call(expr: &Expr<'_>) -> bool {
60+
let ExprKind::Call(callee, args, _) = &expr.kind else { return false };
61+
if is_builtin(callee, kw::Revert) {
62+
return true;
63+
}
64+
if is_require_or_assert(callee)
65+
&& let Some(first) = args.exprs().next()
66+
&& matches!(
67+
&first.peel_parens().kind,
68+
ExprKind::Lit(lit) if matches!(lit.kind, LitKind::Bool(false))
69+
)
70+
{
71+
return true;
72+
}
73+
false
74+
}

crates/lint/src/sol/high/unchecked_calls.rs

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
use super::{UncheckedCall, UncheckedTransferERC20};
22
use crate::{
33
linter::{EarlyLintPass, LateLintPass, LintContext},
4-
sol::{Severity, SolLint, calls::is_low_level_call},
4+
sol::{
5+
Severity, SolLint,
6+
analysis::interface::{is_elementary, receiver_contract_id},
7+
calls::is_low_level_call,
8+
},
59
};
610
use solar::{
711
ast::{Expr, ExprKind, ItemFunction, Stmt, StmtKind, visit::Visit},
@@ -52,13 +56,6 @@ impl<'hir> LateLintPass<'hir> for UncheckedTransferERC20 {
5256
///
5357
/// Validates the method name, the params (count + types), and the returns (count + types).
5458
fn is_erc20_transfer_call(hir: &hir::Hir<'_>, expr: &hir::Expr<'_>) -> bool {
55-
let is_type = |var_id: hir::VariableId, type_str: &str| {
56-
matches!(
57-
&hir.variable(var_id).ty.kind,
58-
hir::TypeKind::Elementary(ty) if ty.to_abi_str() == type_str
59-
)
60-
};
61-
6259
// Ensure the expression is a call to a contract member function.
6360
let hir::ExprKind::Call(
6461
hir::Expr { kind: hir::ExprKind::Member(contract_expr, func_ident), .. },
@@ -77,27 +74,7 @@ fn is_erc20_transfer_call(hir: &hir::Hir<'_>, expr: &hir::Expr<'_>) -> bool {
7774
_ => return false,
7875
};
7976

80-
let Some(cid) = (match &contract_expr.kind {
81-
// Call to pre-instantiated contract variable
82-
hir::ExprKind::Ident([hir::Res::Item(hir::ItemId::Variable(id)), ..]) => {
83-
if let hir::TypeKind::Custom(hir::ItemId::Contract(cid)) = hir.variable(*id).ty.kind {
84-
Some(cid)
85-
} else {
86-
None
87-
}
88-
}
89-
// Call to address wrapped by the contract interface
90-
hir::ExprKind::Call(
91-
hir::Expr {
92-
kind: hir::ExprKind::Ident([hir::Res::Item(hir::ItemId::Contract(cid))]),
93-
..
94-
},
95-
..,
96-
) => Some(*cid),
97-
_ => None,
98-
}) else {
99-
return false;
100-
};
77+
let Some(cid) = receiver_contract_id(hir, contract_expr) else { return false };
10178

10279
// Try to find a function in the contract that matches the expected signature.
10380
hir.contract_item_ids(cid).any(|item| {
@@ -108,8 +85,16 @@ fn is_erc20_transfer_call(hir: &hir::Hir<'_>, expr: &hir::Expr<'_>) -> bool {
10885
&& func.mutates_state()
10986
&& func.parameters.len() == expected_params.len()
11087
&& func.returns.len() == expected_returns.len()
111-
&& func.parameters.iter().zip(expected_params).all(|(id, &ty)| is_type(*id, ty))
112-
&& func.returns.iter().zip(expected_returns).all(|(id, &ty)| is_type(*id, ty))
88+
&& func
89+
.parameters
90+
.iter()
91+
.zip(expected_params)
92+
.all(|(id, &ty)| is_elementary(hir, *id, ty))
93+
&& func
94+
.returns
95+
.iter()
96+
.zip(expected_returns)
97+
.all(|(id, &ty)| is_elementary(hir, *id, ty))
11398
})
11499
}
115100

crates/lint/src/sol/low/missing_zero_check.rs

Lines changed: 16 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
use super::MissingZeroCheck;
22
use crate::{
33
linter::{LateLintPass, LintContext},
4-
sol::{Severity, SolLint},
4+
sol::{
5+
Severity, SolLint,
6+
analysis::primitives::{
7+
address_call_receiver, branch_always_exits, is_address_type, is_require_or_assert,
8+
},
9+
},
510
};
611
use solar::{
712
ast,
8-
interface::{data_structures::Never, kw, sym},
9-
sema::hir::{self, ElementaryType, ExprKind, ItemId, Res, StmtKind, TypeKind, Visit},
13+
interface::data_structures::Never,
14+
sema::hir::{self, ExprKind, ItemId, Res, StmtKind, Visit},
1015
};
1116
use std::{
1217
collections::{HashMap, HashSet},
@@ -32,7 +37,7 @@ impl<'hir> LateLintPass<'hir> for MissingZeroCheck {
3237
}
3338

3439
let params: HashSet<hir::VariableId> =
35-
func.parameters.iter().copied().filter(|id| is_address(hir, *id)).collect();
40+
func.parameters.iter().copied().filter(|id| is_address_type(hir, *id)).collect();
3641

3742
if params.is_empty() {
3843
return;
@@ -70,10 +75,6 @@ fn is_entry_point(func: &hir::Function<'_>) -> bool {
7075
&& matches!(func.visibility, ast::Visibility::Public | ast::Visibility::External)
7176
}
7277

73-
fn is_address(hir: &hir::Hir<'_>, id: hir::VariableId) -> bool {
74-
matches!(hir.variable(id).ty.kind, TypeKind::Elementary(ElementaryType::Address(_)))
75-
}
76-
7778
/// Tracks address-parameter taint, sinks reached, and guards observed in a function body.
7879
struct Analyzer<'hir> {
7980
hir: &'hir hir::Hir<'hir>,
@@ -248,7 +249,7 @@ impl<'hir> Visit<'hir> for Analyzer<'hir> {
248249
StmtKind::DeclSingle(var_id) => {
249250
let v = self.hir.variable(var_id);
250251
if let Some(init) = v.initializer
251-
&& is_address(self.hir, var_id)
252+
&& is_address_type(self.hir, var_id)
252253
{
253254
let srcs = self.taint_sources(init);
254255
if !srcs.is_empty() {
@@ -299,7 +300,7 @@ impl<'hir> Visit<'hir> for Analyzer<'hir> {
299300
}
300301
// Taint propagation: assignment to an address local.
301302
if let Some(local) = lhs_local_var(self.hir, lhs)
302-
&& is_address(self.hir, local)
303+
&& is_address_type(self.hir, local)
303304
{
304305
let srcs = self.taint_sources(rhs);
305306
if !srcs.is_empty() {
@@ -334,60 +335,14 @@ impl<'hir> Visit<'hir> for Analyzer<'hir> {
334335
}
335336
}
336337

337-
fn is_require_or_assert(callee: &hir::Expr<'_>) -> bool {
338-
if let ExprKind::Ident(reses) = &callee.kind {
339-
return reses.iter().any(|r| {
340-
if let Res::Builtin(b) = r {
341-
let n = b.name();
342-
n == sym::require || n == sym::assert
343-
} else {
344-
false
345-
}
346-
});
347-
}
348-
false
349-
}
350-
351-
/// If `callee` is `<receiver>.{call,delegatecall,transfer,send}` (with or without
352-
/// call options), returns the `<receiver>` expression.
353-
fn address_call_receiver<'hir>(callee: &'hir hir::Expr<'hir>) -> Option<&'hir hir::Expr<'hir>> {
354-
// `addr.call{value: x}(..)` lowers as `Call(Member(receiver, "call"), ..)` — peel an
355-
// outer call layer so the inner Member is reachable.
356-
let inner = match &callee.kind {
357-
ExprKind::Call(inner, ..) => inner,
358-
_ => callee,
359-
};
360-
let target = if matches!(inner.kind, ExprKind::Member(..)) { inner } else { callee };
361-
if let ExprKind::Member(receiver, name) = &target.kind {
362-
let n = name.name;
363-
if n == kw::Call || n == kw::Delegatecall || n == sym::transfer || n == sym::send {
364-
return Some(receiver);
365-
}
366-
}
367-
None
368-
}
369-
370-
fn branch_always_exits(stmt: &hir::Stmt<'_>) -> bool {
371-
match &stmt.kind {
372-
StmtKind::Return(_) | StmtKind::Revert(_) => true,
373-
StmtKind::Block(block) | StmtKind::UncheckedBlock(block) => {
374-
block.stmts.last().is_some_and(branch_always_exits)
375-
}
376-
StmtKind::If(_, t, Some(e)) => branch_always_exits(t) && branch_always_exits(e),
377-
_ => false,
378-
}
379-
}
380-
381338
fn is_address_state_var_lhs(hir: &hir::Hir<'_>, lhs: &hir::Expr<'_>) -> bool {
382339
if let ExprKind::Ident(reses) = &lhs.kind {
383340
for res in *reses {
384-
if let Res::Item(ItemId::Variable(vid)) = res {
385-
let v = hir.variable(*vid);
386-
if v.kind.is_state()
387-
&& matches!(v.ty.kind, TypeKind::Elementary(ElementaryType::Address(_)))
388-
{
389-
return true;
390-
}
341+
if let Res::Item(ItemId::Variable(vid)) = res
342+
&& hir.variable(*vid).kind.is_state()
343+
&& is_address_type(hir, *vid)
344+
{
345+
return true;
391346
}
392347
}
393348
}

crates/lint/src/sol/med/incorrect_erc20_interface.rs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::IncorrectERC20Interface;
22
use crate::{
33
linter::{LateLintPass, LintContext},
4-
sol::{Severity, SolLint},
4+
sol::{Severity, SolLint, analysis::interface::is_elementary},
55
};
66
use solar::sema::hir;
77

@@ -68,21 +68,12 @@ fn has_incorrect_erc20_signature(
6868
parameters: &[hir::VariableId],
6969
returns: &[hir::VariableId],
7070
) -> bool {
71-
let is_type = |var_id: hir::VariableId, type_str: &str| {
72-
matches!(
73-
&hir.variable(var_id).ty.kind,
74-
hir::TypeKind::Elementary(ty) if ty.to_abi_str() == type_str
75-
)
76-
};
77-
78-
let params_match = |params: &[hir::VariableId], expected: &[&str]| -> bool {
79-
params.len() == expected.len()
80-
&& params.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
81-
};
82-
83-
let returns_match = |rets: &[hir::VariableId], expected: &[&str]| -> bool {
84-
rets.len() == expected.len() && rets.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
71+
let sig_match = |vars: &[hir::VariableId], expected: &[&str]| -> bool {
72+
vars.len() == expected.len()
73+
&& vars.iter().zip(expected).all(|(&id, &ty)| is_elementary(hir, id, ty))
8574
};
75+
let params_match = sig_match;
76+
let returns_match = sig_match;
8677

8778
match name {
8879
// function transfer(address,uint256) external returns (bool)

crates/lint/src/sol/med/incorrect_erc721_interface.rs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::IncorrectERC721Interface;
22
use crate::{
33
linter::{LateLintPass, LintContext},
4-
sol::{Severity, SolLint},
4+
sol::{Severity, SolLint, analysis::interface::is_elementary},
55
};
66
use solar::sema::hir;
77

@@ -57,21 +57,12 @@ fn has_incorrect_erc721_signature(
5757
parameters: &[hir::VariableId],
5858
returns: &[hir::VariableId],
5959
) -> bool {
60-
let is_type = |var_id: hir::VariableId, type_str: &str| {
61-
matches!(
62-
&hir.variable(var_id).ty.kind,
63-
hir::TypeKind::Elementary(ty) if ty.to_abi_str() == type_str
64-
)
65-
};
66-
67-
let params_match = |params: &[hir::VariableId], expected: &[&str]| -> bool {
68-
params.len() == expected.len()
69-
&& params.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
70-
};
71-
72-
let returns_match = |rets: &[hir::VariableId], expected: &[&str]| -> bool {
73-
rets.len() == expected.len() && rets.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
60+
let sig_match = |vars: &[hir::VariableId], expected: &[&str]| -> bool {
61+
vars.len() == expected.len()
62+
&& vars.iter().zip(expected).all(|(&id, &ty)| is_elementary(hir, id, ty))
7463
};
64+
let params_match = sig_match;
65+
let returns_match = sig_match;
7566

7667
match name {
7768
// function balanceOf(address) external view returns (uint256)

crates/lint/src/sol/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use thiserror::Error;
3737
#[macro_use]
3838
pub mod macros;
3939

40+
pub mod analysis;
4041
mod calls;
4142
pub mod codesize;
4243
pub mod gas;

0 commit comments

Comments
 (0)