search

code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

The transferOwnership function did not check if the newOwner address is 0, resulting in unexpected behavior. #44

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L66-L82 https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L126-L131

Vulnerability details

Impact

The transferOwnership function did not check if the newOwner address is 0, resulting in unexpected behavior.

Proof of Concept

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L66-L82 function transferOwnership( address newOwner ) public virtual override(OwnableUnset, ILSP14Ownable2Step) onlyOwner { _transferOwnership(newOwner);

    address currentOwner = owner();
    emit OwnershipTransferStarted(currentOwner, newOwner);

    newOwner.tryNotifyUniversalReceiver(
        _TYPEID_LSP14_OwnershipTransferStarted,
        ""
    );
    require(
        currentOwner == owner(),
        "LSP14: newOwner MUST accept ownership in a separate transaction"
    );
}

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L126-L131 function _transferOwnership(address newOwner) internal virtual { if (newOwner == address(this)) revert CannotTransferOwnershipToSelf();

    _pendingOwner = newOwner;
    delete _renounceOwnershipStartedAt;
}

Tools Used

vscode

Recommended Mitigation Steps

add check

Assessed type

Other

code423n4 commented 1 year ago

Withdrawn by ziyou-