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

Anyone Can Block LayerZero Channel un/intentionally Due to Absents of Minimum Gas Checking #785

Closed
c4-submissions opened this issue Oct 6, 2023 · 7 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 sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") 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/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L829
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L921
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L776

Vulnerability details

Description

Many function in the Maia Protocol's contracts allow users to send a cross-chain messages with specified gas parameters without any limitation. Where they eventually will be sent through these functions:

  • RootBridgeAgent._performCall
  • RootBridgeAgent._performRetrySettlementCall
  • BranchBridgeAgent._performCall

A potential vulnerability has been identified in multiple functions that using _perform* functions, due to the lack of proper gas validation and enforcement. This vulnerability allows any caller to specify GasParams (used to build LayerZero's Relayer adapterParams) for the cross-chain call, including a potentially low gas value. Without proper gas validation, this could lead to scenarios where cross-chain calls fail due to insufficient gas, potentially causing denial-of-service (DoS) situations or blockage of the message pathway.

External Vulnerable Functions

  1. BranchBridgeAgent contract

    • callOutAndBridge
    • callOutAndBridgeMultiple
    • callOutSigned
    • callOutSignedAndBridge
    • callOutSignedAndBridgeMultiple
    • retryDeposit
    • retrieveDeposit
    • retrySettlement - 2 level cross-chain messages
  2. BaseBranchRouter

    • callOut
  3. RootBridgeAgent contract

    • retrySettlement
    • retrieveSettlement
  4. CoreBranchRouter

    • addGlobalToken - 3 level cross-chain messages
    • addLocalToken
  5. CoreRootRouter

    • addBranchToBridgeAgent - 2 level cross-chain messages
    • toggleBranchBridgeAgentFactory
    • removeBranchBridgeAgent

...

Some of these functions controlled by a trusted entities but this issue could happens unintentionally, and It's important to note that multi level cross-chain calls is allowed! while this vulnerability can be occurs in very complex chain of cross-chain calls.

Impact

In scenarios where a LayerZero cross-chain call fails due to low gas, it could potentially block the message pathway, preventing the normal flow of messages channel between two chains.

Proof of Concept

Explanation:

When the gas limit specified in the try block (inside LayerZero Endpoint) extremely low and cannot cover the gas requirements of the internal calls within lzReceive, It's important to note that while excessivelySafeCall can help prevent reverts due to out-of-gas conditions caused by low-level calls, it cannot overcome extremely low gas limits set at the transaction level. To ensure the successful execution of desired operations, the gas limit should be appropriately set.

Additionally, If the gas limit provided in the try block is insufficient to even enter the lzReceive function, the transaction will revert before the internal call inside lzReceive reaches excessivelySafeCall.

Therefore, if an attacker sets an extremely low gas limit in the try block, and that limit is inadequate to enter the lzReceive function, the error of out-of-gas will be caught before the internal call inside lzReceive reaches the excessivelySafeCall, leading to a failed transaction.

Attack scenario

  1. Attacker's Intent:

    • An attacker seeks to disrupt the Maia Protocol's message pathway or cause a DoS situation.
  2. Low Gas Specification:

    • The attacker calls one of the functions that allow controlling adapterParams, specifying an exceptionally low gas value, for instance, 30,000 gas units.
  3. Cross-Chain Call:

    • The function initiates a cross-chain call with the specified gas value.
  4. Failure to Execute:

    • As a result of the low gas value provided, the cross-chain call fails to execute due to insufficient gas to cover the internal calls within lzReceive, lead to storing the payload in StoredPayload.
  5. Impact:

    • The Maia Protocol experiences a DoS situation, and the channel message pathway may be blocked as the protocol is using NonBlockingLzApp architecture.

Test Case

This test will verify the vulnerability by simulating two scenarios:

  1. Low Gas Attack: It will test the scenario where an attacker specifies an extremely low gas value, causing a cross-chain call to fail due to insufficient gas. This test aims to demonstrate how this vulnerability can lead to a denial-of-service (DoS) situation or blockage of the message pathway.

  2. Normal Gas: It will test the scenario with an appropriate gas value to ensure that the cross-chain call executes successfully, preventing a DoS situation. This test serves as a control to show the expected behavior when using the correct gas parameters.

These test cases aim to demonstrate the impact of gas limitations on the Maia Protocol's message pathway and highlight the importance of proper gas validation and enforcement.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import "forge-std/Test.sol";

import {ExcessivelySafeCall} from "lib/ExcessivelySafeCall.sol";


contract EndpointMock {

    struct StoredPayload {
        uint64 payloadLength;
        address dstAddress;
        bytes32 payloadHash;
    }

    // storedPayload = [srcChainId][srcAddress]
    mapping(uint16 => mapping(bytes => StoredPayload)) public storedPayload;

    event PayloadStored(uint16 srcChainId, bytes srcAddress, address dstAddress, uint64 nonce, bytes payload, bytes reason);

    function receivePayload(uint16 _srcChainId, bytes calldata _srcAddress, address _dstAddress, uint64 _nonce, uint _gasLimit, bytes calldata _payload) external {
        try LzReceiverMock(_dstAddress).lzReceive{gas: _gasLimit}(_srcChainId, _srcAddress, _nonce, _payload) {
            // success, do nothing, end of the message delivery
        } catch (bytes memory reason) {
            // revert nonce if any uncaught errors/exceptions if the ua chooses the blocking mode
            storedPayload[_srcChainId][_srcAddress] = StoredPayload(uint64(_payload.length), _dstAddress, keccak256(_payload));
            emit PayloadStored(_srcChainId, _srcAddress, _dstAddress, _nonce, _payload, reason);
        }
    }
}

contract LzReceiverMock {
    using ExcessivelySafeCall for address;
    uint256 nonce;

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

    function lzReceiveNonBlocking(
        address _endpoint,
        uint16 _srcChainId,
        bytes calldata _srcAddress,
        bytes calldata _payload
    ) public {
        require(_srcChainId == 0, "reverting test");
        _endpoint;
        _srcAddress;
        _payload;
        nonce++;
    }

}
contract LowGasTest is Test {
    EndpointMock endpointMock;
    LzReceiverMock lzReceiver;

    event PayloadStored(uint16 srcChainId, bytes srcAddress, address dstAddress, uint64 nonce, bytes payload, bytes reason);


    constructor() {
        endpointMock = new EndpointMock();
        lzReceiver = new LzReceiverMock();
    }

    function test_PoC_LowGasStoreThePayload() public {
        bytes memory _payload = bytes("dummy");
        bytes memory _srcAddress = bytes("_srcAddress");


        vm.expectEmit(true, true, false, true);
        emit PayloadStored(0, _srcAddress, address(lzReceiver), 0, _payload, bytes(""));
        endpointMock.receivePayload(0, _srcAddress, address(lzReceiver), 0, 10, _payload); // Call lzReceiver with low gas

        (, address payloadSrcAddress,) = endpointMock.storedPayload(0, _srcAddress);
        assertEq(payloadSrcAddress, address(lzReceiver));
    }

    function test_PoC_RightGasDoseNotStorePayloadWhenReverts() public {
        // chainId = 1 to make the lzReceive reverts
        endpointMock.receivePayload(1, bytes("_srcAddress"), address(lzReceiver), 0, 10000, bytes("dummy")); 
    }
}

Tools Used

  • Manual review
  • Foundry

Recommended Mitigation Steps

consider Implementing gas validation and enforcement mechanisms within the function. Ensure that the gas provided by the caller is above a minimum threshold, which should cover the worst-case gas consumption scenario for the cross-chain call. it should hit (bool success,) = address(this).excessivelySafeCall(... when receiving cross-chain messages.

According to (LayerZero Integration Checklist - LayerZero Docs):

Make sure to test the amount of gas required for the execution on the destination. Use custom adapter
parameters and specify minimum destination gas for each cross-chain path when the default amount of
gas (200,000) is not enough. This requires whoever calls the send function to provide the adapter
params with a destination gas >= amount set in the minDstGasLookup for that chain. So that your
users don't run into failed messages on the destination. It makes it a smoother end-to-end experience
for all.

Also this snippet can be used to implement the check from LayerZero implementation of LzApp

function _checkGasLimit(uint16 _dstChainId, uint16 _type, bytes memory _adapterParams, uint _extraGas) internal view virtual {
    uint providedGasLimit = _getGasLimit(_adapterParams);
    uint minGasLimit = minDstGasLookup[_dstChainId][_type] + _extraGas;
    require(minGasLimit > 0, "LzApp: minGasLimit not set");
    require(providedGasLimit >= minGasLimit, "LzApp: gas limit is too low");
}

function _getGasLimit(bytes memory _adapterParams) internal pure virtual returns (uint gasLimit) {
    require(_adapterParams.length >= 34, "LzApp: invalid adapterParams");
    assembly {
        gasLimit := mload(add(_adapterParams, 34))
    }
}

It would be easier to implement the check right before the send call, Also it will prevent the multi level cross-chain calls and break any call that will end up in StoredPayload.

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 c4-pre-sort added the primary issue Highest quality submission among a set of duplicates label Oct 7, 2023
@c4-pre-sort
Copy link

0xA5DF marked the issue as primary issue

@c4-pre-sort
Copy link

0xA5DF marked the issue as sufficient quality report

@0xA5DF
Copy link

0xA5DF commented Oct 12, 2023

Notice #528 claims this can be used to steal the airdropped gas from the messages sent while blocked

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Oct 16, 2023
@c4-sponsor
Copy link

0xLightt (sponsor) confirmed

@c4-judge c4-judge added duplicate-399 and removed primary issue Highest quality submission among a set of duplicates labels Oct 22, 2023
@c4-judge
Copy link
Contributor

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

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

alcueca changed the severity to 2 (Med Risk)

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Oct 22, 2023
@c4-judge
Copy link
Contributor

alcueca marked the issue as satisfactory

This issue was closed.
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 sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

5 participants