sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

ginlee - [M-1]Two Step Transfer #18

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ginlee

medium

[M-1]Two Step Transfer

Summary

consider implementing a Two-Step Transfer pattern when changing owner

Vulnerability Detail

Completing ownership transfer with only one step means that if the current owner of the contract accidentally executes this function, it may transfer ownership to the wrong address. Therefore, in practical applications, the "two-step principle" should be used for safer ownership transfer

Impact

May lose ownership if wrong address is provided

Code Snippet

https://github.com/0xSplits/splits-utils/blob/0dd263bf6feb0bd26b054da3cf1bb742d0bfb13e/src/OwnableImpl.sol#L45-L48

Tool used

Manual Review

Recommendation

implementing a Two-Step Transfer pattern

address $owner address pendingOwner

function transferOwnership(address owner) public virtual onlyOwner { pendingOwner = owner; emit NewOwnershipTransferred(); }

function acceptOwnership() external { require(msg.sender == pendingOwner, "!pendingOwner"); $owner = pendingOwner; emit OwnershipTransferred() pendingOwner = address(0) }