code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

An enforced owner can write existing configs in the faucets. For instance, when bridge operations is in-progress, owner can change all configs. That will leads to locked funds.

Proof of Concept

Navigate to the following contract.

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

Enforced owner can initialize function multiple times. Existing config can be overwritten by an owner multiple times.

    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;
    }

Tools Used

Code Review

Recommended Mitigation Steps

Ensure that Facet multiple initialization methodology is intended.

maxklenk commented 2 years ago

Yes, it is intend to be able to initialize a bridge multiple time to be able to update them if the implementation of the bridges updates. This will not influence single bridging transaction as they are only forwarded in our contract and will still be executed by the underlying bridge even if we update the address later to another one.

gzeoneth commented 2 years ago

Downgrading to Low/QA.