Skip to content

Commit e68ee66

Browse files
authored
Merge branch 'master' into mattsse/delegatecall-loop-followups
2 parents 3993543 + 621f493 commit e68ee66

7 files changed

Lines changed: 1637 additions & 0 deletions

File tree

crates/lint/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ It helps enforce best practices and improve code quality within Foundry projects
2727
- `block-timestamp`: Warns when `block.timestamp` is used in a comparison, as it may be manipulated by validators.
2828
- `delegatecall-loop`: Payable functions should not use `delegatecall` inside a loop.
2929
- `missing-zero-check`: Address parameter is used in a state write or value transfer without a zero-address check.
30+
- `return-bomb`: External calls with a gas limit should not consume unbounded return data.
3031
- **Informational / Style Guide:**
3132
- `boolean-equal`: Boolean comparisons to constants should be simplified.
3233
- `too-many-digits`: Numeric literals with 5+ consecutive zeros are error-prone.

crates/lint/docs/return-bomb.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Return bomb
2+
3+
**Severity**: `Low`
4+
**ID**: `return-bomb`
5+
6+
Flags external calls that set an explicit gas limit while copying unbounded dynamic returndata.
7+
8+
## What it does
9+
10+
Detects low-level `call`, `delegatecall`, and `staticcall` expressions that specify `{gas: ...}`.
11+
Solidity copies the full returndata for these calls even when the second tuple element is ignored.
12+
It also detects high-level external calls with `{gas: ...}` that consume dynamically encoded return
13+
values such as `bytes`, `string`, dynamic arrays, or structs containing dynamic fields.
14+
15+
## Why is this bad?
16+
17+
The gas option limits gas forwarded to the callee, but copying returndata into memory is paid by
18+
the caller after the call returns. A malicious callee can return or revert with large returndata,
19+
causing the caller to run out of gas while implicitly copying the result.
20+
21+
## Example
22+
23+
### Bad
24+
25+
```solidity
26+
function callTarget(address target, bytes memory payload, uint256 gasLimit) external {
27+
(bool ok, ) = target.call{gas: gasLimit}(payload);
28+
require(ok);
29+
}
30+
```
31+
32+
### Good
33+
34+
```solidity
35+
function callTarget(address target, bytes memory payload, uint256 gasLimit) external {
36+
bool ok;
37+
assembly {
38+
ok := call(gasLimit, target, 0, add(payload, 0x20), mload(payload), 0, 0)
39+
}
40+
require(ok);
41+
}
42+
```
43+
44+
Ignoring the second tuple element does not avoid the copy. If the returndata is needed, copy only a
45+
bounded number of bytes with a helper that caps returndata size.

crates/lint/src/sol/calls.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use solar::{
22
ast::{Expr, ExprKind},
33
interface::kw,
4+
sema::hir,
45
};
56

67
/// Checks if an expression is a low-level call.
@@ -23,3 +24,21 @@ pub(crate) const fn is_low_level_call(expr: &Expr<'_>) -> bool {
2324
}
2425
false
2526
}
27+
28+
/// Checks if a HIR expression is any call with an explicit gas option.
29+
pub(crate) fn is_call_with_gas_limit(expr: &hir::Expr<'_>) -> bool {
30+
let Some((_, opts)) = call_with_options(expr) else {
31+
return false;
32+
};
33+
34+
opts.iter().any(|opt| opt.name.name == kw::Gas)
35+
}
36+
37+
fn call_with_options<'hir>(
38+
expr: &'hir hir::Expr<'hir>,
39+
) -> Option<(&'hir hir::Expr<'hir>, &'hir [hir::NamedArg<'hir>])> {
40+
let hir::ExprKind::Call(callee, _, Some(opts)) = &expr.peel_parens().kind else {
41+
return None;
42+
};
43+
Some((callee, opts))
44+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@ use delegatecall_loop::DELEGATECALL_LOOP;
99
mod missing_zero_check;
1010
use missing_zero_check::MISSING_ZERO_CHECK;
1111

12+
mod return_bomb;
13+
use return_bomb::RETURN_BOMB;
14+
1215
register_lints!(
1316
(BlockTimestamp, early, (BLOCK_TIMESTAMP)),
1417
(DelegatecallLoop, late, (DELEGATECALL_LOOP)),
1518
(MissingZeroCheck, late, (MISSING_ZERO_CHECK)),
19+
(ReturnBomb, late, (RETURN_BOMB)),
1620
);

0 commit comments

Comments
 (0)