diff --git a/sidebar/sidebar.ts b/sidebar/sidebar.ts index 29df6dc17..0e0a8527a 100644 --- a/sidebar/sidebar.ts +++ b/sidebar/sidebar.ts @@ -54,6 +54,7 @@ const docs = [ { text: "erc20-unchecked-transfer", link: "/forge/linting/erc20-unchecked-transfer" }, { text: "incorrect-exp", link: "/forge/linting/incorrect-exp" }, { text: "incorrect-shift", link: "/forge/linting/incorrect-shift" }, + { text: "protected-vars", link: "/forge/linting/protected-vars" }, { text: "reentrancy-eth", link: "/forge/linting/reentrancy-eth" }, { text: "rtlo", link: "/forge/linting/rtlo" }, { text: "unchecked-call", link: "/forge/linting/unchecked-call" }, diff --git a/src/pages/forge/linting.mdx b/src/pages/forge/linting.mdx index 061f9001e..474c922d2 100644 --- a/src/pages/forge/linting.mdx +++ b/src/pages/forge/linting.mdx @@ -163,6 +163,7 @@ navigation on the right to browse by severity. - [`erc20-unchecked-transfer`](/forge/linting/erc20-unchecked-transfer) — Flags calls to ERC20 `transfer` and `transferFrom` where the boolean return value is ignored. - [`incorrect-exp`](/forge/linting/incorrect-exp) — Flags `^` (bitwise xor) used between integer literals where `**` (exponentiation) was almost certainly intended. - [`incorrect-shift`](/forge/linting/incorrect-shift) — Flags shift operations where a literal appears on the left and a non-literal on the right, which is almost always the wrong operand order. +- [`protected-vars`](/forge/linting/protected-vars) — Flags externally callable functions that can write an annotated state variable without invoking its declared protection. - [`reentrancy-eth`](/forge/linting/reentrancy-eth) — Flags uncapped ETH-transferring low-level `call` operations when state read before the call is written after the call. - [`rtlo`](/forge/linting/rtlo) — Flags the presence of Unicode bidirectional override characters in source code, which can be used to hide malicious behavior ("Trojan Source", [CVE-2021-42574](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574)). - [`unchecked-call`](/forge/linting/unchecked-call) — Flags low-level calls (`call`, `delegatecall`, `staticcall`, `callcode`) whose `success` return value is ignored. diff --git a/src/pages/forge/linting/protected-vars.mdx b/src/pages/forge/linting/protected-vars.mdx new file mode 100644 index 000000000..c3ed44460 --- /dev/null +++ b/src/pages/forge/linting/protected-vars.mdx @@ -0,0 +1,79 @@ +--- +description: State writes that bypass declared protection +--- + +## Protected variables + +**Severity**: `High` +**ID**: `protected-vars` + +Flags externally callable functions that can write a state variable without invoking the function +or modifier named by its `@custom:security write-protection` annotation. + +### What it does + +A state variable can declare a required protection with an exact function or modifier signature: + +```solidity +/// @custom:security write-protection="onlyOwner()" +address owner; +``` + +The lint follows modifiers, library calls, and resolved internal call paths from public, external, +fallback, and receive entry points. If an entry point writes the variable directly or through a +reachable helper, the required function or modifier must also be reachable from that entry point. +This includes writes through storage references, returned storage locations, collection +`push`/`pop` operations, and resolvable inline-assembly storage slots. + +Overloads are matched by their exact signature. Inherited variables, entry points, functions, and +modifiers are resolved in the most-derived contract, including virtual dispatch. External calls +such as `this.onlyOwner()` do not satisfy an internal write-protection requirement. An unresolved +signature or malformed `write-protection` value is treated as unsatisfied so an invalid annotation +cannot silently disable the lint. + +Like Slither's annotation semantics, this rule is reachability-based rather than order- or +path-sensitive: the annotation identifies a required internal call, not a proof that the call +dominates every write. + +### Why is this bad? + +Writing security-sensitive state without its declared access check can let an untrusted caller +change ownership, authorization, or other protected configuration. + +### Example + +#### Bad + +```solidity +contract Registry { + /// @custom:security write-protection="onlyOwner()" + address public owner; + + modifier onlyOwner() { + require(msg.sender == owner); + _; + } + + function setOwner(address newOwner) external { + owner = newOwner; + } +} +``` + +#### Good + +```solidity +contract Registry { + /// @custom:security write-protection="onlyOwner()" + address public owner; + + modifier onlyOwner() { + require(msg.sender == owner); + _; + } + + function setOwner(address newOwner) external onlyOwner { + owner = newOwner; + } +} +```