Summary

Single-step Ownership Transfer Can be Dangerous

Vulnerability Details

Single-step ownership transfer means that if a wrong address was passed when transferring ownership or admin rights it can mean that role is lost forever. If the admin permissions are given to the wrong address within this function, it will cause irreparable damage to the contract.

The control of contract ownership is implemented similarly to the Ownable contract from OpenZeppelin. However, it should be noted that the OpenZeppelin documentation mentions that this approach is not secure.

https://docs.openzeppelin.com/contracts/4.x/api/access

"Ownable is a simpler mechanism with a single owner "role" that can be assigned to a single account. This simpler mechanism can be useful for quick tests but projects with production concerns are likely to outgrow it."

Impact

Allowing the protocol to cause irreparable losses results in the loss of ownership and destruction.

Tools Used

Manual review

Recommendations

It is a best practice to use a two-step ownership transfer pattern, meaning ownership transfer gets to a "pending" state and the new owner should claim his new rights, otherwise the old owner still has control of the contract.You can refer to OpenZeppelin's implementation for reference.

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