Two step ownership transfer is recommended

[hats-bug-reporter[bot]]

Github username: @0xfuje Submission hash (on-chain): 0x956c6258fbfd26ae5848ff4889785da7248c02f4b8f9fb5ec1167b9b47d50baa Severity: low

Description:

Description

Most of the contracts use the OwnableUpgradeable helper contract from openzeppelin. The problem is the transferOwnership() function of the library immediately sets the contract’s new owner. Transferring ownership in one step is error-prone and could result in irrevocable mistakes if the wrong address is passed in by mistake.

openzeppelin-contracts-upgradeable/access/OwnableUpgradeable.sol

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

Recommendation

It's best practice to implement a two-step process to transfer contract ownership, in which the owner proposes a new address, then the new address can execute a call to accept their proposed new ownership. Instead of OwnableUpgradeable consider using Ownable2StepUpgradeable

    function transferOwnership(address newOwner) public virtual override onlyOwner {
        _pendingOwner = newOwner;
        emit OwnershipTransferStarted(owner(), newOwner);
    }

    function acceptOwnership() public virtual {
        address sender = _msgSender();
        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
        _transferOwnership(sender);
    }

[seongyun-ko]

TY for suggestion. but it's not an attack vector