code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

Factory.sol inherits Openzeppelin's Ownable contract which is used to restrict certain functions to a specific owner account. By default, the owner account is set to the deployer of Factory.sol. However, the owner can transfer ownership of this contract by calling transferOwnership() with the newOwner as input. If the current owner of the contract mistakenly transfers ownership to an account that is not actively controlled, the ownership of Factory.sol will be lost.

Proof of Concept

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable.sol

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider implementing a proper transfer ownership pattern whereby the current owner nominates a new owner. This new owner then has to claim ownership for the change to be applied. This can be done by overriding the current Ownable implementation of transferOwnership() and adding a claimOwnership() function alongside it.

frank-beard commented 2 years ago

not an exploit but a good point to bring up. it is assumed the owner is trusted.

GalloDaSballo commented 2 years ago

Similarly to #185 I agree with the finding but will downgrade to Low

code-423n4 / 2021-09-defiprotocol-findings