code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

There is no zero address check in `transferOwnership` function in `OwnershipFacet.sol` #192

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/OwnershipFacet.sol#L8-L11

Vulnerability details

Impact

There is no zero address check in transferOwnership function. If a zero address is submitted by any possibility, this may lead the funds to be locked and making them unrecoverable for the users.

Proof of Concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/OwnershipFacet.sol#L8-L11

    function transferOwnership(address _newOwner) external override {
        LibDiamond.enforceIsContractOwner();
        LibDiamond.setContractOwner(_newOwner);
    }

Tools Used

Static testing

Recommended Mitigation Steps

Adding require(_newOwner != address(0)) statement

H3xept commented 2 years ago

Assets are not at direct risk, functionality and availability could be. Fixed in lifinance/lifi-contracts@221fe883f705635feebb2af64a028f30d05afbf8

gzeoneth commented 2 years ago

Downgrading to Low/QA

gzeoneth commented 2 years ago

Consider with #188