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

An Attacker Can Exploit and Steal all Branch Bridge Agents Native Gas Deposits #528

Closed
c4-submissions opened this issue Oct 5, 2023 · 13 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 edited-by-warden sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Oct 5, 2023

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L276-L304
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgentExecutor.sol#L167-L192
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L312

Vulnerability details

Impact

In This report I demonstrate how an attacker can steal unsuspecting users native gas deposit to themselves.

For this attack to succeed, the attacker must obstruct communication between the Root Bridge Agent and the target branch bridge agent.

To execute this attack, the attacker must craft a payload designed to trigger a call within the Root Bridge Agent. This call will inadvertently invoke the branch bridge agent, effectively blocking communication between the Root Bridge Agent contract and the branch bridge agent. This can be achieved by setting a very low gas parameter value(~1), causing an Out-of-Gas (OOG) revert at the BranchBridgeAgent::lzReceive function. Consequently, this action impedes the passage from the root chain to the target chain BranchBridgeAgent contract due to the payload being stored within Layer Zero end point.

To execute the attack, the attacker initiates a call to BranchBridgeAgent::callOutSignedAndBridge.
What basically happens is, the attacker will deposit some underlying tokens on the target branch bridge agent contract, then get global hTokens in the root bridge agent, which with added params(settlement call), burns the attacker's global hTokens to mint local hTokens or clear deposited underlying tokens to the attacker in the branch bridge agent/target branch chain.
The sequence of calls involved in this process is as follows:

BranchBridgeAgent::callOutSignedAndBridge --> lzEndPoint --> RootBridgeAgent::lzReceive --> RootBridgeAgent::lzReceiveNonBlocking --> RootBridgeAgentExecutor::executeSignedWithDeposit --> MulticallRootRouter::executeSignedDepositSingle --> _approveAndCallOut --> RootBridgeAgent::callOutAndBridge --> _createSettlement --> lzEndPoint

The ultimate objective is to allow unsuspecting users to attempt bridging from other branch bridge agent contract to the blocked bridge agent. When this occurs, the native gas sent along with the call for execution(Layer zero fee) at the BranchBridgeAgent chain, will be airdropped into the branch bridge agent, while their call is blocked/reverted at layer zero during relay.
Most users will still invoke functions like RootBridgeAgent::retrySettlement and RootBridgeAgent::retrieveSettlement, sending native gas tokens along for execution(i.e. layer zero fees).

When the attacker observes that the contract has accumulated a sufficient balance, they can then retry the payload, this time sending enough gas to cover the execution cost.

    function retryPayload(uint16 _srcChainId, bytes calldata _srcAddress, bytes calldata _payload) external override receiveNonReentrant {
        StoredPayload storage sp = storedPayload[_srcChainId][_srcAddress];
        require(sp.payloadHash != bytes32(0), "LayerZero: no stored payload");
        require(_payload.length == sp.payloadLength && keccak256(_payload) == sp.payloadHash, "LayerZero: invalid payload");

        address dstAddress = sp.dstAddress;
        // empty the storedPayload
        sp.payloadLength = 0;
        sp.dstAddress = address(0);
        sp.payloadHash = bytes32(0);

        uint64 nonce = inboundNonce[_srcChainId][_srcAddress];

        ILayerZeroReceiver(dstAddress).lzReceive(_srcChainId, _srcAddress, nonce, _payload);
        emit PayloadCleared(_srcChainId, _srcAddress, nonce, dstAddress);
    }

Let us further observe how BranchBridgeAgent handles create settlement request:
BranchBridgeAgent::lzReceiveNonBlocking --> BranchBridgeAgentExecutor::executeWithSettlement

   function executeWithSettlement(address _recipient, address _router, bytes calldata _payload)
        external
        payable
        onlyOwner
    {
        // Clear Token / Execute Settlement
        SettlementParams memory sParams = SettlementParams({
            settlementNonce: uint32(bytes4(_payload[PARAMS_START_SIGNED:PARAMS_TKN_START_SIGNED])),
            recipient: _recipient,
            hToken: address(uint160(bytes20(_payload[PARAMS_TKN_START_SIGNED:45]))),
            token: address(uint160(bytes20(_payload[45:65]))),
            amount: uint256(bytes32(_payload[65:97])),
            deposit: uint256(bytes32(_payload[97:PARAMS_SETTLEMENT_OFFSET]))
        });

        // Bridge In Assets
        BranchBridgeAgent(payable(msg.sender)).clearToken(
            sParams.recipient, sParams.hToken, sParams.token, sParams.amount, sParams.deposit
        );

        // Execute Calldata if there is any
        if (_payload.length > PARAMS_SETTLEMENT_OFFSET) {
            // Execute remote request
            IRouter(_router).executeSettlement{value: msg.value}(_payload[PARAMS_SETTLEMENT_OFFSET:], sParams);
        } else {
            // Send reamininig native / gas token to recipient
            _recipient.safeTransferETH(address(this).balance);
        }
    }

We can observe in the else block that all of the contract balance is sent to the recipient as gas refund upon completion, the recipient used here is the refundee passed in as a parameter, we can observe this here:

    function executeSignedDepositSingle(bytes calldata encodedData, DepositParams calldata, address userAccount, uint16)
        external
        payable
        override
        requiresExecutor
        lock
    {

            /// FUNC ID: 2 (multicallSingleOutput)
        } else if (funcId == 0x02) {
            // Decode Params
            (Call[] memory calls, OutputParams memory outputParams, uint16 dstChainId, GasParams memory gasParams) =
                abi.decode(_decode(encodedData[1:]), (Call[], OutputParams, uint16, GasParams));

            // Make requested calls
            IVirtualAccount(userAccount).call(calls);

            // Withdraw assets from Virtual Account
            IVirtualAccount(userAccount).withdrawERC20(outputParams.outputToken, outputParams.amountOut);

            // Bridge Out assets
            _approveAndCallOut(
                IVirtualAccount(userAccount).userAddress(), // <--HERE
                outputParams.recipient,
                outputParams.outputToken,
                outputParams.amountOut,
                outputParams.depositOut,
                dstChainId,
                gasParams
            );

            /// FUNC ID: 3 (multicallMultipleOutput)
        } else if (funcId == 0x03) {
            // Decode Params

    }

We can observe the revert in BranchBridgeAgent::lzRecieve function here, add this test to BranchBridgeAgentTest.t.sol:

 function testPossible1GasRevert() public {
    uint256 _amount = 10 ether;
    uint256 _deposit = 5 ether;

        //GasParams
        GasParams memory gasParams = GasParams(0.5 ether, 0.5 ether);
        //attacker's passed in gas limit
        uint256 gasLimit = 1; 

        address _recipient = address(this);

        vm.deal(localPortAddress, 1 ether);

        vm.startPrank(localPortAddress);

        // Mint Test tokens.
        ERC20hTokenBranch localhToken = new ERC20hTokenBranch(
                   "Test Ulysses ",
            "test-u",
            "Hermes omni token",
            "hUNDER",
            18,
            localPortAddress
        );
        localhToken.mint(_recipient, _amount - _deposit);

        MockERC20 underToken = new MockERC20("u token", "U", 18);
        underToken.mint(_recipient, _deposit);

        vm.stopPrank();

        // Perform deposit
        makeTestCallWithDeposit(_recipient, address(localhToken), address(underToken), _amount, _deposit, gasParams);

        // Encode Settlement Data for Clear Token Execution
        bytes memory settlementData = abi.encodePacked(
            bytes1(0x01), _recipient, uint32(1), address(localhToken), address(underToken), _amount, _deposit, bytes("")
        );

        // layer zero will call the branch agent with the passed in gasLimit(1), since it isn't controlled, we can pass in one here, thereby
        // causing a revert before the call gets to bridge agent non blocking, which will make the call revert to layer zero
        // layer zero will then catch the error and store the message
        vm.prank(lzEndpointAddress);
        vm.expectRevert();
        bAgent.lzReceive{gas: gasLimit}(rootChainId, abi.encodePacked(bAgent, rootBridgeAgentAddress), 1, settlementData);

    }

Summary

  1. Attacker blocks communication channel between Root Bridge Agent and Branch Bridge Agent.
  2. Users attempt bridging from one branch bridge agent to the blocked bridge agent via root bridge agent.
  3. Layer Zero airdrops Native gas tokens from the user into the blocked branch bridge agent.
  4. Native gas tokens accumulates in the blocked BranchBridgeAgent contract
  5. Attacker retries the deposit with sufficient gas, stealing all of the users previously sent native tokens

Tools Used

Layer Zero Endpoint contracts, Foundry and Research/Manual Review

Recommended Mitigation Steps

Consider adding a check in the contract that ensures users input Gas Params are within a certain threshold.
The set threshold should be enough to ensure the gas limit always gets the call to lzReceiveNonBlocking
Rigorous testing should be done before using this value, the max possible payload size should be considered.

Assessed type

Other

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

0xA5DF marked the issue as duplicate of #785

@c4-pre-sort
Copy link

0xA5DF marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Oct 12, 2023
@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)

@alcueca
Copy link

alcueca commented Oct 22, 2023

retryPayload can be executed by anyone. While this is an interesting attack vector, it doesn't necessarily result in a large amount of funds stolen, and it still amounts to a griefing attack.

@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

@Tendency001
Copy link

Thanks for the great work ser.
I would like to express my thoughts here, A class of issues describes how gas is stuck on revert(#464 ), another class describes how communication channels can be blocked/DOSsed(#399 ), and then this issue describes how an attacker can steal the stuck gas.
This issue showcases the possible DOS and how a well-coined payload will allow an attacker to steal the sent-in gas.

I messaged the sponsors about the former issue(#464 ), I initially thought it would be judged as a QA, so I had to come up with how it can be abused, I then submitted #529 and this, but both issues are currently tagged as one.

I would also like to add that, I wrote this issue, having in mind that anyone can get the stored payload from the layerZero endpoint and resubmit the payload with higher gas to ensure it succeeds and unblock the message channel, so I described how regardless of who retries the payload, the attacker receives the gas refund.
I do believe this issue should be of its own class, thank you.

@alcueca
Copy link

alcueca commented Oct 31, 2023

Upon closer inspection, this attack won't yield any profits to the attacker, as payloads (and their gas) won't be received when the endpoint is blocked.

@c4-judge
Copy link
Contributor

alcueca marked the issue as not a duplicate

@c4-judge c4-judge reopened this Oct 31, 2023
@c4-judge c4-judge added unsatisfactory does not satisfy C4 submission criteria; not eligible for awards and removed satisfactory satisfies C4 submission criteria; eligible for awards labels Oct 31, 2023
@c4-judge
Copy link
Contributor

alcueca marked the issue as unsatisfactory:
Invalid

@Tendency001
Copy link

Thank you ser, I have seen my wrong

@Tendency001
Copy link

Since the users gas tokens will be stuck with the relayer in the destination chains,
Should this issue still be considered as invalid ser?

@alcueca
Copy link

alcueca commented Nov 1, 2023

Let's not go into a fishing expedition in here. This report was for a further attack that would yield significant additional damage over the DoS you already reported in #399. During such DoS, user gas remains in the relayer until the DoS is resolved. That is still the same DoS and the same impact, user transactions taking a while to fully resolve after consuming gas is a common event during cross-chain operations, as you probably know from bridging tokens around in other platforms.

@Tendency001
Copy link

Thanks for the taking the time to respond.
Thanks for the judging.

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 edited-by-warden sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

6 participants