Lack of sanity check in ProtocolDAO.upgradeExistingContract may cause the guardian to delete a contract instead of upgrading it #465
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
duplicate-742
satisfactory
satisfies C4 submission criteria; eligible for awards
Lines of code
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L209-L217
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L198-L203
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L190-L194
Vulnerability details
Impact
Guardian may accidentally delete a contract. Might waste gas and also cause confusion to the users and the guardian himself.
Proof of Concept
When the protocol decides to upgrade its contract, it calls upgradeExistingContract in ProtocolDAO.sol. The upgradeExistingContract calls registerContract and registers a new address and name, then unregisterContract to delete the old contract address and name.
If the guardian accidentally uses the same new address for both new and existing addresses, then there will be nothing to upgrade.
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L209-L217
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L198-L203
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L190-L194
Recommended Mitigation Steps
Make sure newAddr is not equal to existing Addr and newAddr is not the same as current Addr.
The text was updated successfully, but these errors were encountered: