|
| 1 | +""" |
| 2 | +Detector for potentially unsafe uses of address.balance. |
| 3 | +
|
| 4 | +Detects: |
| 5 | +1. Strict equality comparisons (== or !=) with address.balance |
| 6 | +2. Assignment of address.balance to state variables |
| 7 | +
|
| 8 | +These patterns are problematic because: |
| 9 | +- Attackers can forcibly send ETH using selfdestruct |
| 10 | +- Pre-calculated contract addresses can receive ETH before deployment |
| 11 | +- Balance can change between transactions unpredictably |
| 12 | +
|
| 13 | +Related to issue #2778. |
| 14 | +""" |
| 15 | + |
| 16 | +from __future__ import annotations |
| 17 | + |
| 18 | +from slither.analyses.data_dependency.data_dependency import is_dependent_ssa |
| 19 | +from slither.core.cfg.node import Node |
| 20 | +from slither.core.declarations import Function |
| 21 | +from slither.core.declarations.contract import Contract |
| 22 | +from slither.core.declarations.function_contract import FunctionContract |
| 23 | +from slither.core.declarations.function_top_level import FunctionTopLevel |
| 24 | +from slither.core.declarations.solidity_variables import SolidityFunction |
| 25 | +from slither.core.variables.state_variable import StateVariable |
| 26 | +from slither.detectors.abstract_detector import ( |
| 27 | + AbstractDetector, |
| 28 | + DetectorClassification, |
| 29 | + DETECTOR_INFO, |
| 30 | +) |
| 31 | +from slither.slithir.operations import ( |
| 32 | + Assignment, |
| 33 | + Binary, |
| 34 | + BinaryType, |
| 35 | + SolidityCall, |
| 36 | +) |
| 37 | +from slither.slithir.variables.temporary_ssa import TemporaryVariableSSA |
| 38 | +from slither.slithir.variables.local_variable import LocalIRVariable |
| 39 | +from slither.utils.output import Output |
| 40 | + |
| 41 | + |
| 42 | +class BalanceReliance(AbstractDetector): |
| 43 | + """ |
| 44 | + Detects potentially unsafe uses of address.balance: |
| 45 | + 1. Strict equality comparisons (== or !=) |
| 46 | + 2. Assignment to state variables |
| 47 | + """ |
| 48 | + |
| 49 | + ARGUMENT = "balance-reliance" |
| 50 | + HELP = "Dangerous reliance on address.balance" |
| 51 | + IMPACT = DetectorClassification.LOW |
| 52 | + CONFIDENCE = DetectorClassification.MEDIUM |
| 53 | + |
| 54 | + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#balance-reliance" |
| 55 | + WIKI_TITLE = "Dangerous reliance on address.balance" |
| 56 | + |
| 57 | + WIKI_DESCRIPTION = """ |
| 58 | +Detects potentially unsafe uses of `address.balance`: |
| 59 | +1. Strict equality comparisons (`==` or `!=`) - An attacker can forcibly send ETH using `selfdestruct`, breaking equality assumptions. |
| 60 | +2. Assignment to state variables - Storing balance values leads to stale data and incorrect assumptions.""" |
| 61 | + |
| 62 | + WIKI_EXPLOIT_SCENARIO = """ |
| 63 | +```solidity |
| 64 | +contract Crowdsale { |
| 65 | + uint256 public savedBalance; |
| 66 | +
|
| 67 | + function fund_reached() public returns(bool) { |
| 68 | + // BAD: strict equality can be manipulated |
| 69 | + return address(this).balance == 100 ether; |
| 70 | + } |
| 71 | +
|
| 72 | + function saveBalance() public { |
| 73 | + // BAD: balance can change, making stored value stale |
| 74 | + savedBalance = address(this).balance; |
| 75 | + } |
| 76 | +} |
| 77 | +``` |
| 78 | +An attacker can use `selfdestruct` to forcibly send ETH to the contract, making `fund_reached()` return false even after 100 ETH is collected. Similarly, `savedBalance` becomes stale immediately after being set.""" |
| 79 | + |
| 80 | + WIKI_RECOMMENDATION = """ |
| 81 | +Use range checks instead of strict equality for balance comparisons: |
| 82 | +```solidity |
| 83 | +// GOOD: use >= for minimum balance checks |
| 84 | +require(address(this).balance >= 100 ether, "Insufficient balance"); |
| 85 | +
|
| 86 | +// GOOD: use range checks |
| 87 | +require(address(this).balance >= minAmount && address(this).balance <= maxAmount); |
| 88 | +``` |
| 89 | +Avoid storing balance in state variables. If needed, recalculate on each use.""" |
| 90 | + |
| 91 | + # Only applicable to Solidity |
| 92 | + LANGUAGE = "solidity" |
| 93 | + |
| 94 | + def _find_balance_taints( |
| 95 | + self, functions: list[FunctionContract] |
| 96 | + ) -> list[LocalIRVariable | TemporaryVariableSSA]: |
| 97 | + """ |
| 98 | + Find all variables that hold address.balance values. |
| 99 | + """ |
| 100 | + taints = [] |
| 101 | + for func in functions: |
| 102 | + for node in func.nodes: |
| 103 | + for ir in node.irs_ssa: |
| 104 | + if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( |
| 105 | + "balance(address)" |
| 106 | + ): |
| 107 | + taints.append(ir.lvalue) |
| 108 | + return taints |
| 109 | + |
| 110 | + def _is_tainted( |
| 111 | + self, |
| 112 | + var, |
| 113 | + taints: list[LocalIRVariable | TemporaryVariableSSA], |
| 114 | + contract: Contract, |
| 115 | + ) -> bool: |
| 116 | + """Check if a variable is tainted by address.balance.""" |
| 117 | + for taint in taints: |
| 118 | + if is_dependent_ssa(var, taint, contract): |
| 119 | + return True |
| 120 | + return False |
| 121 | + |
| 122 | + def _detect_strict_equality( |
| 123 | + self, |
| 124 | + functions: list[FunctionContract], |
| 125 | + taints: list[LocalIRVariable | TemporaryVariableSSA], |
| 126 | + contract: Contract, |
| 127 | + ) -> list[tuple[FunctionContract, Node, str]]: |
| 128 | + """ |
| 129 | + Detect strict equality comparisons (== or !=) with balance-tainted values. |
| 130 | + """ |
| 131 | + results = [] |
| 132 | + for func in functions: |
| 133 | + if isinstance(func, FunctionTopLevel): |
| 134 | + continue |
| 135 | + for node in func.nodes: |
| 136 | + for ir in node.irs_ssa: |
| 137 | + if isinstance(ir, Binary) and ir.type in ( |
| 138 | + BinaryType.EQUAL, |
| 139 | + BinaryType.NOT_EQUAL, |
| 140 | + ): |
| 141 | + # Check if either operand is tainted by balance |
| 142 | + left_tainted = self._is_tainted(ir.variable_left, taints, contract) |
| 143 | + right_tainted = self._is_tainted(ir.variable_right, taints, contract) |
| 144 | + if left_tainted or right_tainted: |
| 145 | + op = "==" if ir.type == BinaryType.EQUAL else "!=" |
| 146 | + results.append((func, node, f"strict equality ({op})")) |
| 147 | + return results |
| 148 | + |
| 149 | + def _detect_state_assignment( |
| 150 | + self, |
| 151 | + functions: list[FunctionContract], |
| 152 | + taints: list[LocalIRVariable | TemporaryVariableSSA], |
| 153 | + contract: Contract, |
| 154 | + ) -> list[tuple[FunctionContract, Node, str]]: |
| 155 | + """ |
| 156 | + Detect assignment of balance-tainted values to state variables. |
| 157 | + """ |
| 158 | + results = [] |
| 159 | + for func in functions: |
| 160 | + if isinstance(func, FunctionTopLevel): |
| 161 | + continue |
| 162 | + for node in func.nodes: |
| 163 | + for ir in node.irs_ssa: |
| 164 | + if isinstance(ir, Assignment): |
| 165 | + # Check if assigning to a state variable |
| 166 | + if isinstance(ir.lvalue, StateVariable) or ( |
| 167 | + hasattr(ir.lvalue, "non_ssa_version") |
| 168 | + and isinstance(ir.lvalue.non_ssa_version, StateVariable) |
| 169 | + ): |
| 170 | + # Check if the value being assigned is tainted |
| 171 | + if self._is_tainted(ir.rvalue, taints, contract): |
| 172 | + results.append((func, node, "state variable assignment")) |
| 173 | + return results |
| 174 | + |
| 175 | + def _detect(self) -> list[Output]: |
| 176 | + results = [] |
| 177 | + |
| 178 | + for contract in self.compilation_unit.contracts_derived: |
| 179 | + functions = contract.all_functions_called + contract.modifiers |
| 180 | + |
| 181 | + # Find all balance taints |
| 182 | + taints = self._find_balance_taints(functions) |
| 183 | + if not taints: |
| 184 | + continue |
| 185 | + |
| 186 | + # Detect strict equality comparisons |
| 187 | + equality_issues = self._detect_strict_equality(functions, taints, contract) |
| 188 | + |
| 189 | + # Detect state variable assignments |
| 190 | + assignment_issues = self._detect_state_assignment(functions, taints, contract) |
| 191 | + |
| 192 | + # Combine and report |
| 193 | + all_issues = equality_issues + assignment_issues |
| 194 | + |
| 195 | + for func, node, issue_type in all_issues: |
| 196 | + info: DETECTOR_INFO = [ |
| 197 | + func, |
| 198 | + f" uses address.balance in {issue_type}:\n", |
| 199 | + "\t- ", |
| 200 | + node, |
| 201 | + "\n", |
| 202 | + ] |
| 203 | + results.append(self.generate_result(info)) |
| 204 | + |
| 205 | + return results |
0 commit comments