code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

Airdropped Gas will remain in the Agent in case of failure #887

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L579-L583

Vulnerability details

Impact

The protocol uses LayerZeros Airdrop mechanism to send gas to BridgeAgents which they need to pay for subsequential cross-chain-messages. If the transaction on the receiver fails, this airdropped gas will remain in the BridgeAgent and can be used up by the next caller.

Proof of Concept

The use of the Airdrop mechanism can be seen for example in BranchBridgeAgent._performCall:

    function _performCall(address payable _refundee, bytes memory _payload, GasParams calldata _gParams)
        internal
        virtual
    {
        ILayerZeroEndpoint(lzEndpointAddress).send{value: msg.value}(
            ...
            abi.encodePacked(uint16(2), _gParams.gasLimit, _gParams.remoteBranchExecutionGas, rootBridgeAgentAddress)
        );
    }

An example that shows an attempt of using the airdropped gas to perform a follow up call is in BranchBridgeAgent._performFallBackCall:

ILayerZeroEndpoint(lzEndpointAddress).send{value: address(this).balance}(...

This call happens after the execution of another call has failed, for example _executeWithSettlement:

function executeWithSettlement(address _recipient, address _router, bytes calldata _payload)
    external
    payable
    onlyOwner
{
    ...
    } else {
        _recipient.safeTransferETH(address(this).balance);
    }
}

As can be seen, the function attempts to send back the remaining native gas to the recipient. If the execution fails, the fallback is triggered. However the fallback too can fail, if the gas balance of the bridge agent is not enough to cover the relayer costs (looking at LayerZero code -> UltraLightNodeV2.send gets called by the Endpoint). In this case, the airdropped gas is not send anywhere and can be used up by the next caller.

Tools Used

Manual Review

Recommended Mitigation Steps

At lzReceive (which can never fail due to a low level call, with additional safety), the contract balance could be sent to a refundee or recipient in case of failure:

    function lzReceive(uint16, bytes calldata _srcAddress, uint64, bytes calldata _payload) public override {
        address(this).excessivelySafeCall(
            gasleft(),
            150,
            abi.encodeWithSelector(this.lzReceiveNonBlocking.selector, msg.sender, _srcAddress, _payload)
        );

        //check if !success and transfer address(this).balance to the recipient (has to be determined from payload)
    }

Assessed type

Other

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-sponsor commented 1 year ago

0xBugsy (sponsor) confirmed

c4-judge commented 1 year ago

alcueca marked issue #728 as primary and marked this issue as a duplicate of 728

c4-judge commented 1 year ago

alcueca marked the issue as satisfactory

c4-judge commented 1 year ago

alcueca marked the issue as duplicate of #518