MainP1 contract inherits from UUPSUpgradeable and overrides the _authorizeUpgrade() function. However, without adding access control to this function, anyone can call it to set the implementation contract. This lack of restriction allows an attacker to set a malicious contract as the implementation, which can then use the selfdestruct() function to destroy the proxy contract. This is possible because the implementation is called using delegatecall from the proxy.
Proof of Concept
function _authorizeUpgrade(address) internal view override {
Tools Used
Manual Review
Recommended Mitigation Steps
Add proper access control to _authorizeUpgrade() function since it is used for authorization at the time of changing implementation contract address. This can be mitigated by using 'OwnableUpgradeable' of openzeppelin and add onlyOwner modifier to the _authorizeUpgrade() function so. Only owner can be authorized to upgrade the implementation address.
Lines of code
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Main.sol#L153
Vulnerability details
Impact
MainP1contract inherits fromUUPSUpgradeableand overrides the_authorizeUpgrade()function. However, without adding access control to this function, anyone can call it to set the implementation contract. This lack of restriction allows an attacker to set a malicious contract as the implementation, which can then use theselfdestruct()function to destroy the proxy contract. This is possible because the implementation is called using delegatecall from the proxy.Proof of Concept
Tools Used
Manual Review
Recommended Mitigation Steps
Add proper access control to
_authorizeUpgrade()function since it is used for authorization at the time of changing implementation contract address. This can be mitigated by using 'OwnableUpgradeable' of openzeppelin and addonlyOwnermodifier to the_authorizeUpgrade()function so. Only owner can be authorized to upgrade the implementation address.Assessed type
Access Control