code423n4 commented 2 years ago

Lines of code

Vulnerability details

Impact

All the funds and functions in LiFi would become inaccessible if contractOwner was accidentally or maliciously set to zero.

Proof of Concept

function setContractOwner(address _newOwner) internal {
        DiamondStorage storage ds = diamondStorage();
        address previousOwner = ds.contractOwner;
        ds.contractOwner = _newOwner;
        emit OwnershipTransferred(previousOwner, _newOwner);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a zero address check in setContractOwner:

function setContractOwner(address _newOwner) internal {
        require(_newOwner != address(0), "Zero address not valid")
        DiamondStorage storage ds = diamondStorage();
        address previousOwner = ds.contractOwner;
        ds.contractOwner = _newOwner;
        emit OwnershipTransferred(previousOwner, _newOwner);
    }

H3xept commented 2 years ago

Duplicate of #192

gzeoneth commented 2 years ago

Downgrading to Low/QA