improvements to zero address check detector
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
follow on https://github.com/crytic/slither/pull/1043 https://github.com/crytic/slither/pull/1033
I think the usage of the data dependency here:
https://github.com/crytic/slither/blob/191d6d4e661cd14c715f0f7b86593b69037322c3/slither/detectors/operations/missing_zero_address_validation.py#L81-L82
Will lead to miss TP. For example the following code:
```solidity
contract C{
address owner;
modifier onlyOwner(){
require(msg.sender == owner);
_;
}
function set(address addr) public{
require(msg.sender == owner);
owner = addr;
}
}
```
Would raise a warning before the PR, and would not with these changes.
I am wondering also if we could not refactor the detector's logic - it becomes complex to follow.
Maybe we can mark all the `Constant(0)` usage as source of dependency, and check for any `==`/`!=` operations where the left variable is a parameter, and the right variable is dependent on one of the `Constant(0)` we detected?
_Originally posted by @montyly in https://github.com/crytic/slither/issues/1033#issuecomment-1034916789_
Contributor guide
Assessment
This issue has not been assessed yet.