c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BaseBranchRouter.sol#L88-L101 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BaseBranchRouter.sol#L104-L116

Vulnerability details

Impact

In a Re-entrancy attack, a malicious contract calls back into the calling contract before the first invocation of the function is finished. This may cause the different invocations of the function to interact in undesirable ways, especially in cases where the function is updating state variables after the external calls. This may lead to loss of funds, improper value updates, token loss, etc.

Proof of Concept

The callOutAndBridge function is vulnerable to reentrancy

// Line 88-101
    function callOutAndBridge(bytes calldata _params, DepositInput calldata _dParams, GasParams calldata _gParams)
        external
        payable
        override
        lock
    {
        //Transfer tokens to this contract.
        _transferAndApproveToken(_dParams.hToken, _dParams.token, _dParams.amount, _dParams.deposit);

        //Perform call to bridge agent.
        IBridgeAgent(localBridgeAgentAddress).callOutAndBridge{value: msg.value}(
            payable(msg.sender), _params, _dParams, _gParams
        );
    }

The callOutAndBridgeMultiple function is vulnerable to reentrancy

// 104-116
    function callOutAndBridgeMultiple(
        bytes calldata _params,
        DepositMultipleInput calldata _dParams,
        GasParams calldata _gParams
    ) external payable override lock {
        //Transfer tokens to this contract.
        _transferAndApproveMultipleTokens(_dParams.hTokens, _dParams.tokens, _dParams.amounts, _dParams.deposits);

        //Perform call to bridge agent.
        IBridgeAgent(localBridgeAgentAddress).callOutAndBridgeMultiple{value: msg.value}(
            payable(msg.sender), _params, _dParams, _gParams
        );
    }

Tools Used

VS Code.

Recommended Mitigation Steps

It is recommended to add a [Re-entrancy Guard] to the functions making external calls. The functions should use a Checks-Effects-Interactions pattern. The external calls should be executed at the end of the function and all the state-changing must happen before the call.