diff --git a/sidebar/sidebar.ts b/sidebar/sidebar.ts index b5cb8c4d0..24909dde7 100644 --- a/sidebar/sidebar.ts +++ b/sidebar/sidebar.ts @@ -130,6 +130,7 @@ const docs = [ { text: "pascal-case-struct", link: "/forge/linting/pascal-case-struct" }, { text: "pragma-inconsistent", link: "/forge/linting/pragma-inconsistent" }, { text: "redundant-base-constructor-call", link: "/forge/linting/redundant-base-constructor-call" }, + { text: "reentrancy-unlimited-gas", link: "/forge/linting/reentrancy-unlimited-gas" }, { text: "screaming-snake-case-const", link: "/forge/linting/screaming-snake-case-const" }, { text: "screaming-snake-case-immutable", link: "/forge/linting/screaming-snake-case-immutable" }, { text: "too-many-digits", link: "/forge/linting/too-many-digits" }, diff --git a/src/pages/forge/linting.mdx b/src/pages/forge/linting.mdx index 7775e2891..f19133736 100644 --- a/src/pages/forge/linting.mdx +++ b/src/pages/forge/linting.mdx @@ -228,6 +228,7 @@ navigation on the right to browse by severity. - [`pascal-case-struct`](/forge/linting/pascal-case-struct) — Flags struct definitions whose names do not follow `PascalCase`. - [`pragma-inconsistent`](/forge/linting/pragma-inconsistent) — Flags projects whose source files declare incompatible or differently-shaped Solidity version pragmas. - [`redundant-base-constructor-call`](/forge/linting/redundant-base-constructor-call) — Flags explicit empty base-constructor arguments (e.g. `is A()` or `constructor() A() {}`) when the base contract requires no arguments. +- [`reentrancy-unlimited-gas`](/forge/linting/reentrancy-unlimited-gas) — Flags state changes and event emissions that remain reachable after built-in `transfer` or `send` calls. - [`screaming-snake-case-const`](/forge/linting/screaming-snake-case-const) — Flags `constant` state variables whose names do not follow `SCREAMING_SNAKE_CASE`. - [`screaming-snake-case-immutable`](/forge/linting/screaming-snake-case-immutable) — Flags `immutable` state variables whose names do not follow `SCREAMING_SNAKE_CASE`. - [`too-many-digits`](/forge/linting/too-many-digits) — Flags numeric literals containing five or more consecutive zeros, which are easy to misread. diff --git a/src/pages/forge/linting/reentrancy-unlimited-gas.mdx b/src/pages/forge/linting/reentrancy-unlimited-gas.mdx new file mode 100644 index 000000000..a2e83f91a --- /dev/null +++ b/src/pages/forge/linting/reentrancy-unlimited-gas.mdx @@ -0,0 +1,63 @@ +--- +description: Reentrancy through fixed-gas ETH transfers +--- + +## Reentrancy through fixed-gas ETH transfers + +**Severity**: `Info` +**ID**: `reentrancy-unlimited-gas` + +Flags state changes and event emissions that occur after Solidity's built-in `transfer` or `send` +on the same reachable path. These operations forward a fixed gas stipend, but opcode repricing can +make new callback behavior affordable within that stipend. + +### What it does + +The lint tracks Solidity's built-in `transfer` and `send` calls through public and external entry +points, modifiers, inherited helpers, and other internal calls. It follows branches and loop +backedges and warns when a later reachable operation writes contract state or emits an event. +Writes through storage references and storage-array `push`/`pop` calls count as state changes. +Both calls are tracked regardless of the transferred amount because even a zero-value call +executes recipient code. + +For `send` used in a Boolean control-flow condition, only continuations on which the call may have +succeeded are treated as reentrant because a failed call reverts its callee's effects. Sibling +expression order is conservatively treated as unspecified, matching Solidity's evaluation-order +rules. + +Contract methods that merely reuse the names `transfer` or `send` are excluded by checking the +compiler-resolved call target. Low-level calls, including calls with an explicit 2,300 gas cap, are +outside this detector's Slither-compatible scope. Constructors, local-variable writes, effects +that occur only before the call, and effects on mutually exclusive paths are not reported. + +### Why is this bad? + +The fixed stipend is not a stable reentrancy boundary. Changes to EVM opcode costs can alter which +fallback operations fit within it, so code that assumes `transfer` or `send` can never call back may +become unsafe after a network upgrade. A callback before a later state write can observe stale +state, while a callback before a later event can reorder logs consumed by off-chain systems. + +Apply checks-effects-interactions: commit state and emit its corresponding event before interacting +with the recipient. Use a reentrancy guard when the interaction cannot safely be moved last. + +### Example + +#### Bad + +```solidity +function withdraw(address payable recipient, uint256 amount) external { + recipient.transfer(amount); + balances[recipient] -= amount; + emit Withdrawal(recipient, amount); +} +``` + +#### Good + +```solidity +function withdraw(address payable recipient, uint256 amount) external { + balances[recipient] -= amount; + emit Withdrawal(recipient, amount); + recipient.transfer(amount); +} +``` diff --git a/vercel.json b/vercel.json index 66cda9a85..b49df6301 100644 --- a/vercel.json +++ b/vercel.json @@ -53,7 +53,6 @@ { "source": "/forge/reference/forge-lint/", "destination": "/forge/linting" }, { "source": "/forge/reference/forge-lint", "destination": "/forge/linting" }, - { "source": "/forge/linting/reentrancy-unlimited-gas", "destination": "/forge/linting/reentrancy-eth" }, { "source": "/cast/overview", "destination": "/cast" },