incorrect-equality: gate on callee mutability to suppress view/pure helper FPs
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the desired feature
`incorrect-equality` fires on strict equalities inside `view`/`pure` callees, but the WIKI's exploit (attacker grieves a state transition by manipulating `block.timestamp` / `block.number` / a balance) needs the comparison to actually gate a state write.
`tainted_equality_nodes` at `slither/detectors/statements/incorrect_strict_equality.py:147-150` only filters `FunctionTopLevel`:
```python
for func in funcs:
if isinstance(func, FunctionTopLevel):
continue
for node in func.nodes:
...
```
No mutability check, so a side-effect-free callee still gets reported.
Repro:
```solidity
contract C {
uint256 public immutable deployBlock;
constructor() { deployBlock = block.number; }
function isDeployBlock() external view returns (bool) {
return block.number == deployBlock; // reported; no state to grief
}
}
```
### The possible solution
Add a mutability gate after the `FunctionTopLevel` check:
```python
if getattr(func, "view", False) or getattr(func, "pure", False):
continue
```
That kills the FP above. But it also kills a real case: an `internal view` helper called inside a state-modifying caller, like
```solidity
function _isExpired() internal view returns (bool) {
return block.timestamp == deadline;
}
function withdraw() external {
if (_isExpired()) { /* release funds */ }
}
```
slither's taint doesn't cross function boundaries, so the warning won't transfer to `withdraw`. That's a real recall loss on a pattern that's pretty common.
### Alternatives we've considered
1. Only skip `public` / `external view`. Narrower, but `_isExpired()` is `internal` so still missed.
2. Call-graph reachability. Skip a `view`/`pure` callee only if no state-modifying function in the contract transitively reaches it. One pass from state-modifying entry points.
3. No code change. Document the boundary in the WIKI, let users `// slither-disable-next-line` on confirmed FPs.
### Additional context
The strict-equality findings in #2425 (`CreateX._parseSalt`, `_requireSuccessfulContractCreation`, `_guard`, all `internal pure` / `internal view`) are this FP class.
#2759 is a different sub-class: magic-number compares (`== 0`, `== type(uint256).max`) inside stateful functions. Not about callee mutability, needs a separate value-class filter.
Contributor guide
Assessment
This issue has not been assessed yet.