CoreBranchRouter.executeNoSettlement() fails to refund the unconsumed gas after completion of execution

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/CoreBranchRouter.sol#L86-L149
https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgentExecutor.sol#L53-L56
https://github.com/code-423n4/2023-09-maia/blob/main/src/MulticallRootRouter.sol#L142-L149
https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgentExecutor.sol#L72
https://github.com/code-423n4/2023-09-maia/blob/main/src/MulticallRootRouter.sol#L234-L241
https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgentExecutor.sol#L156

Vulnerability details

CoreBranchRouter.executeNoSettlement() is used to execute branch management functions such as addGlobalToken, addBridgeAgent, manageStrategyToken, etc. It receives native tokens from BranchBridgeAgentExecutor for gas to perform subsequent callout to root chain. In the case of addGlobalToken and addBridgeAgent, the callout to root chain will take care of the refund of native tokens as LayerZeroEndpoint(lzEndpointAddress).send() will refund the excess minus fees.

However, for management functions that does not have subsequent callout like manageStrategyToken, removeBranchBridgeAgent, the native tokens are not refunded by CoreBranchRouter after successful execution. The unconsumed native tokens will be left stuck in the CoreBranchRouter contract.

Note that the issue is possible as CoreRootRouter allows the passing in of GasParams to allow transfer of native tokens for remote execution gas. And this issue occurs in the test suite as shown in the POC below.

The issue is also present in ArbitrumCoreBranchRouter.executeNoSettlement(), MulticallRootRouter.execute(), MulticallRootRouter.executeSigned(). These functions accept native tokens for remote execution but has specific cases that does not perform subsequent callouts.

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgentExecutor.sol#L53-L56

contract BranchBridgeAgentExecutor is Ownable, BridgeAgentConstants {
    ...
    function executeNoSettlement(address _router, bytes calldata _payload) external payable onlyOwner {
        
        //@audit CoreBranchRouter will receive native tokens for gas to perform subsequent callout to root chain
        IRouter(_router).executeNoSettlement{value: msg.value}(_payload[PARAMS_TKN_START_SIGNED:]);
    }
}

Impact

The unconsumed gas will be left stuck within the router contracts without any mechanism to retrieve them.

Proof of Concept

Add the console.log as shown below to existing testAddStrategyToken() in RootTest.t.sol#L747. It will show that the CoreBranchRouter will be left with the unconsumed native tokens.

function testAddStrategyToken() public {
    // Get some gas
    hevm.deal(address(this), 1 ether);

    rootCoreRouter.manageStrategyToken{value: 0.05 ether}(
        address(102), 3000, address(this), ftmChainId, GasParams(0.05 ether, 0.05 ether)
    );

    require(ftmPort.isStrategyToken(address(102)), "Should be added");

    //@audit add the following console.log to see the unrefunded gas that is left in the destination branch router. 
    console.log("address(ftmCoreRouter).balance == %d", address(ftmCoreRouter).balance);
}

Recommended Mitigation Steps

Implement a refund by transfering the native tokens that was received from msg.value.

Assessed type

Token-Transfer

[c4-pre-sort]

0xA5DF marked the issue as sufficient quality report

[c4-pre-sort]

0xA5DF marked the issue as primary issue

[c4-sponsor]

0xLightt (sponsor) disputed

[alcueca]

Disputed on what grounds, please?

[0xLightt]

This would require API misuse by governance. The governance contract would be sending extra native tokens to be used for further layerzero/cross-chain calls when there are none expected. The gas parameters passed should be something like this GasParams(0.05 ether, 0). Since the "airdrop" functionality of layerzero adapter params has no use here.

[c4-judge]

alcueca marked the issue as unsatisfactory:
Invalid