Summary

Single-step process for critical ownership transfer

Vulnerability Details

The current ownership transfer process involves the current owner calling transferOwnership(). This function have one step ownership transfer and it doesn't check that the new owner is not the zero address and proceeds to write the new owner's address into the owner's state variable. If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, breaking all functions with the onlyOwner() modifier.

Issue Location

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/utils/Ownable.sol#L19-L22

Impact

If an incorrect address, e.g. for which the private key is not known, is used accidentally then it prevents the use of all the onlyOwner() functions forever, which includes the changing of various deployer contract addresses and market approvals. This use of incorrect address may not even be immediately apparent given that these functions are probably not used immediately. When noticed, due to a failing onlyOwner() function call, it will force the redeployment of the factory contract and require appropriate changes and notifications for switching from the old to new address. This will diminish trust in markets and incur a significant reputational damage.

Tools Used

Manual Review

Recommendations

use a two-step ownership transfer pattern and add a zero-address check.