update unprotected-upgradeable to consider whether selfdestruct functions can be called directly
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
Uninitialized UUPS contracts that use OZ >=4.3.2 will have FPs because the functions which delegatecall (`upgradeTo` and `upgradeToAndCall`) must be [called within the context of deletatecall](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/91e8d0ba3c3beb8a1db31310d8599664e48639ef/contracts/proxy/utils/UUPSUpgradeable.sol#L68-L86) (they have an `onlyProxy` modifer). Thus, the UUPS implementation cannot be destroyed although there may be other undesirable consequeunces.
The detector should consider whether the functions found in `_can_be_destroyed` have this restriction:
https://github.com/crytic/slither/blob/4c976d5af56219eeef079e03a35009af3e03644d/slither/detectors/statements/unprotected_upgradeable.py#L9
Ref https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories/GHSA-5vp3-v4hc-gx76
Contributor guide
Research direction
Start in slither/detectors/statements/unprotected_upgradeable.py, especially _can_be_destroyed, and review the linked OpenZeppelin UUPSUpgradeable implementation and its onlyProxy restriction. The detector should distinguish directly callable selfdestruct paths from upgrade functions that require delegatecall context, with the false positive described in the issue no longer reported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, solidity
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 50/100