Picodes
commented
1 year ago QA for me: this is a design choice. Also if the owner is compromised, the attacker can already modify all parameters without changing the ownership, so I don't really see what the timelock would prevent in this case.
c4-judge
Lines of code
https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/OwnableTwoSteps.sol#L98-L105 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/OwnableTwoSteps.sol#L83-L92
Vulnerability details
Impact
Ownership compromise is unforeseeable. The system has implemented some measures to mitigate the impact, e.g. a seperate
ERC20EnabledLooksRareAggregator. One of the reasons of ownership compromise is private info leakage (e.g. account privatekey).Proof of Concept
Ther are may be various reasons leading to private info leakage, such as:
Tools Used
Manual audit.
Recommended Mitigation Steps
When an attacker compromise the ownership, oen of the important things the attacker will do is to transfer the current ownership to the attacker's hand. Once the ownership is successfully transferred, the original owner won't be able to intervene the attacker. It is vital to take some measures to prevent the attacker from successfully transferring the ownership so as for the original owner to transfer the ownership to a secured owner. Some measures can be taken to prevent the attacker from successfully transferring the ownership in this situation: In the Contract
OwnableTwoSteps, add an extra fixed waiting period for ownership transfer, say 7 days, only after this period, thenewPotentialOwnercan take over. The company should actively monitor the smart contract events. If anyInitiateOwnershipTransferevent is not intended, the current owner can investigate the incident and cancel the ownership transfer if needed.