code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L49

Vulnerability details

Impact

During the code review, It has been observed that to init function is lack of necessary check on the HopFacet contract. Below code , we can clearly see tokens and _bridgeConfigs array size is not checked and If they are different that will cause to revert.

    function initHop(
        string[] memory _tokens,
        IHopBridge.BridgeConfig[] memory _bridgeConfigs,
        uint256 _chainId
    ) external {
        Storage storage s = getStorage();
        LibDiamond.enforceIsContractOwner();

        for (uint8 i; i < _tokens.length; i++) {
            s.hopBridges[_tokens[i]] = _bridgeConfigs[i];
        }
        s.hopChainId = _chainId;
    }

Proof of Concept

Navigate to the following contract.

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L49

Tools Used

Code Review

Recommended Mitigation Steps

Ensure that tokens and _bridgeConfigs mapping sizes are checked at the beginning of the function.

maxklenk commented 2 years ago

Thanks for pointing out this possible check. As this a admin function and it would already revert if invalid data is passed this is not really a problem. For convenience we will add a check and a nice revert message but do disagree with the severity. This is a QA Report.

H3xept commented 2 years ago

Fixed in lifinance/lifi-contracts@f35ed79a266a69b363d72332b7861d15d18b98cb

gzeoneth commented 2 years ago

Downgrading to Low/QA.