Skip to content

Commit 073ea8c

Browse files
0xKarl98mattsse
andauthored
feat(lint): add delegatecall-loop lint (#14752)
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
1 parent 342301d commit 073ea8c

8 files changed

Lines changed: 869 additions & 5 deletions

File tree

crates/lint/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ It helps enforce best practices and improve code quality within Foundry projects
2525
- `unused-return`: Return value of an external call is not used.
2626
- **Low Severity:**
2727
- `block-timestamp`: Warns when `block.timestamp` is used in a comparison, as it may be manipulated by validators.
28+
- `delegatecall-loop`: Payable functions should not use `delegatecall` inside a loop.
2829
- `missing-zero-check`: Address parameter is used in a state write or value transfer without a zero-address check.
2930
- **Informational / Style Guide:**
3031
- `boolean-equal`: Boolean comparisons to constants should be simplified.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Delegatecall inside payable loop
2+
3+
**Severity**: `Low`
4+
**ID**: `delegatecall-loop`
5+
6+
Flags `delegatecall` operations inside loops in externally callable payable functions.
7+
8+
## What it does
9+
10+
Reports `delegatecall` expressions that appear in the body of a `for`, `while`, or `do while`
11+
loop when the enclosing function is `public payable` or `external payable`.
12+
13+
## Why is this bad?
14+
15+
`delegatecall` executes another contract's code in the caller's storage context and preserves
16+
the original `msg.sender` and `msg.value`. In a payable function, a loop can therefore expose the
17+
same `msg.value` to multiple delegatecalls even though Ether was only transferred once.
18+
19+
If the delegated code accounts for `msg.value`, one transaction can credit the same payment
20+
multiple times or repeatedly mutate the caller's storage in unexpected ways.
21+
22+
## Example
23+
24+
### Bad
25+
26+
```solidity
27+
function batch(address[] calldata receivers) external payable {
28+
for (uint256 i; i < receivers.length; ++i) {
29+
address(this).delegatecall(abi.encodeWithSignature("credit(address)", receivers[i]));
30+
}
31+
}
32+
```
33+
34+
### Good
35+
36+
```solidity
37+
function batch(address[] calldata receivers) external payable {
38+
uint256 share = msg.value / receivers.length;
39+
for (uint256 i; i < receivers.length; ++i) {
40+
_credit(receivers[i], share);
41+
}
42+
}
43+
```
44+
45+
## Notes
46+
47+
Review each occurrence manually. If `delegatecall` is required, ensure delegated code cannot
48+
reuse `msg.value` or unexpectedly modify caller storage across loop iterations.

crates/lint/src/linter/late.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use solar::{interface::data_structures::Never, sema::hir};
1+
use solar::{
2+
interface::data_structures::Never,
3+
sema::{Gcx, hir},
4+
};
25
use std::ops::ControlFlow;
36

47
use super::LintContext;
@@ -62,6 +65,15 @@ pub trait LateLintPass<'hir>: Send + Sync {
6265
_func: &'hir hir::Function<'hir>,
6366
) {
6467
}
68+
fn check_function_with_gcx(
69+
&mut self,
70+
ctx: &LintContext,
71+
_gcx: Gcx<'hir>,
72+
hir: &'hir hir::Hir<'hir>,
73+
func: &'hir hir::Function<'hir>,
74+
) {
75+
self.check_function(ctx, hir, func);
76+
}
6577
fn check_modifier(
6678
&mut self,
6779
_ctx: &LintContext,
@@ -110,6 +122,7 @@ pub trait LateLintPass<'hir>: Send + Sync {
110122
pub struct LateLintVisitor<'a, 's, 'hir> {
111123
ctx: &'a LintContext<'s, 'a>,
112124
passes: &'a mut [Box<dyn LateLintPass<'hir> + 's>],
125+
gcx: Gcx<'hir>,
113126
hir: &'hir hir::Hir<'hir>,
114127
}
115128

@@ -120,9 +133,10 @@ where
120133
pub fn new(
121134
ctx: &'a LintContext<'s, 'a>,
122135
passes: &'a mut [Box<dyn LateLintPass<'hir> + 's>],
136+
gcx: Gcx<'hir>,
123137
hir: &'hir hir::Hir<'hir>,
124138
) -> Self {
125-
Self { ctx, passes, hir }
139+
Self { ctx, passes, gcx, hir }
126140
}
127141
}
128142

@@ -183,7 +197,7 @@ where
183197

184198
fn visit_function(&mut self, func: &'hir hir::Function<'hir>) -> ControlFlow<Self::BreakValue> {
185199
for pass in self.passes.iter_mut() {
186-
pass.check_function(self.ctx, self.hir, func);
200+
pass.check_function_with_gcx(self.ctx, self.gcx, self.hir, func);
187201
}
188202
self.walk_function(func)
189203
}
@@ -389,7 +403,7 @@ mod tests {
389403
);
390404
let mut passes: Vec<Box<dyn LateLintPass<'_>>> =
391405
vec![Box::new(RecordingPass { counts: counts.clone() })];
392-
let mut visitor = LateLintVisitor::new(&ctx, &mut passes, &gcx.hir);
406+
let mut visitor = LateLintVisitor::new(&ctx, &mut passes, gcx, &gcx.hir);
393407
let _ = hir::Visit::visit_nested_source(&mut visitor, source_id);
394408
Ok(())
395409
})

0 commit comments

Comments
 (0)