Lack of a double-step `transferOwnership()` pattern

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/tree/main/src/libraries/OracleLib.sol#L3https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/tree/main/src/DecentralizedStableCoin.sol#L39-L67

Summary

Vulnerability Details

The current ownership transfer process for all the contracts inheriting from Ownable or OwnableUpgradeable involves the current owner calling the transferOwnership() function:

    function transferOwnership(address newOwner) public virtual onlyOwner {
        require(newOwner != address(0), "Ownable: new owner is the zero address");
        _transferOwnership(newOwner);
    }

If the nominated EOA account is not a valid account, it is entirely possible that the owner may accidentally transfer ownership to an uncontrolled account, losing the access to all functions with the onlyOwner modifier.

There is 1 instance of this issue:

DecentralizedStableCoin does not implement a 2-Step-Process for transferring ownership.

Impact

Tools Used

Recommendations

It is recommended to implement a two-step process where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of the ownership to fully succeed. This ensures the nominated EOA account is a valid and active account. This can be easily achieved by using OpenZeppelin’s Ownable2Step contract instead of Ownable:

abstract contract Ownable2Step is Ownable {

    /**
     * @dev Starts the ownership transfer of the contract to a new account. Replaces the pending transfer if there is one.
     * Can only be called by the current owner.
     */
    function transferOwnership(address newOwner) public virtual override onlyOwner {
        _pendingOwner = newOwner;
        emit OwnershipTransferStarted(owner(), newOwner);
    }

    ...

    /**
     * @dev The new owner accepts the ownership transfer.
     */
    function acceptOwnership() external {
        address sender = _msgSender();
        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
        _transferOwnership(sender);
    }
}

Cyfrin / 2023-07-foundry-defi-stablecoin