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

25 stars 17 forks source link

Mishandling of settlement operation in MulticallRootRouter leads to complete user fund theft by an attacker #398

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/RootBridgeAgent.sol#L987

Vulnerability details

Impact

This finding can allow an attacker to steal ALL of the user funds deposited into the protocol.

Proof of Concept

I attached PoC code at the bottom.

To be able to explain the attack, I first need to explain the execution flow of deposits and settlements and how they work.

Imagine a user in Avalanche chain wants to deposit 1,000 USDC tokens to the protocol. Deposit essentially means that the user tokens are moved from the branch to the root chain. The user's underlying 1,000 USDC tokens will be taken from him, locked in the Branch's port and he will be given/mintedminted 1,000 global hUSDC tokens sitting on the root.

User will first have to create a DepositInput object, specifying the address of the underlying token he wants to spend and the address of the local branch chain representation of that underlying token hToken and using MultiCall he will then call the callOutAndBridge() function in BaseBranchRouter or CoreBranchRouter or BranchBridgeAgent and supply to the function the DepositInput which he has created. Keep in mind that the user has to approve any of those 3 components to spend his 1,000 underlying USDC for this operation to work.

If the user called the callOutAndBridge() function in BaseBranchRouter or CoreBranchRouter. They will simply transfer the tokens from the user to the Branch's router and approve the branch's port to spend those tokens on the router's behalf. If the user called the callOutAndBridge() function in BranchBridgeAgent then the user will have to approve the branch's port to spend those tokens on his behalf. The user calls this function in any of the components and what happens next is that the BranchPort simply takes those 1,000 underlying USDC from the user and a request to the BranchBridgeAgent is sent, asking it to reach out to RootBridgeAgent to mint global 1,000 hUSDC tokens.

⚠️ First important note. Those 1,000 global hUSDC tokens minted will be owned by the Root Router MulticallRootRouter (same goes for CoreRootRouter). They will not be owned by the user but rather the Root Router. And at root level, there isn't any mechanism to tell if those 1,000 global hUSDC tokens belong to the user. They are owned by the Root's router now, and only it can move those tokens anywhere. And that's exactly what the attacker will exploit

When it comes to settlements. Settlements are same as deposits but instead of tokens moving from the branch to root like deposits, it works the other way around, tokens are moved from Root to Branch. If the user wants his tokens back, he will have to execute a settlement operation. After the user initiates it, a request from branch bridge agent will be sent to the root asking it to unlock the user's 1,000 underlying tokens from the branch port and burn the 1,000 global tokens burnt. The root receives the request, burns 1,000 global hUSDC tokens and sends a request to the branch asking it to "clear" or "unlock" the user's underlying 1,000 USDC tokens which were locked inside the branch port

The Vulnerability

The attacker now wants to steal the user's funds, so he will need to execute a settlement operation:

  1. He will first have to create a payload with a OutputParams object which will be needed for initiating the settlement process in the MulticallRouter. The OutputParams object needs 4 things:

    • The recipient, this tells the root router who to move tokens to. The attacker will specify himself.
    • The outputToken, this is the global token address. The attacker will supply the address of the global hUSDC token.
    • amountOut, the total amount of tokens that the requester wants to receive. The attacker will specify 1000.
    • depositOut, this is the amount of underlying tokens the requester wants to receive. The attacker will specify 1000
  2. The attacker will create this OutputParams object, add it to the payload and add the destination branch chain ID which will receive the underlying USDC to the payload, in this case it's the chain ID of avalanche (since that's where the user had his 1,000 underlying USDC in the first place), and the attacker will add the function ID "2" to his payload to trigger this conditional in MulticallRootRouter

  3. Attacker will send this payload to the function callOut in BaseBranchRouter or BranchBridgeAgent which will simply add the deposit flag "1" to the payload and then the request will be sent to root

  4. Now we are in RootBridgeAgent, In lzReceiveNonBlocking function, this conditional will be triggered because the deposit flag of the payload was "1"

  5. RootBridgeAgent will trigger the executeNoDeposit function in RootBridgeAgentExecutor and hand it the payload.

  6. The RootBridgeAgentExecutor will call the execute function in MulticallRootRouter and hand it the payload

  7. The execute function in MulticallRootRouter will be executed and the second conditional would be triggered becuase our payload had function ID "2".

  8. In the execute function, a dummy multicall specified by the attacker in the payload will be sent and then the execute function will parse the OutputParams object the attacker created and will call the internal function _approveAndCallOut supplied with the following values as parameters:

    • "refundee" as the attacker's address
    • "recipient" as the attacker's address
    • "outputToken" as the global hUSDC token address
    • "amountOut" as 1,000
    • "depositOut" as 1,000
    • "dstChainId" as Avalanche chain ID
  9. Inside the _approveAndCallOut function, the MulticallRouter (which holds the global minted tokens) will approve the root bridge agent to spend those tokens, check line: https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L521

  10. callOutAndBridge will be executed in the RootBridgeAgent (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L175) and it will call the internal function createSettlement and supply it with the following values as arguments:

    • "_settlementNonce" will be the value of the state variable "_settlementNonce"
    • "_refundee" as the attacker's address
    • "_recipient" as the attacker's address
    • "_dstChainId" as the avalanche chain id
    • "_params" as empty
    • "_globalAddress" as the global hUSDC token address
    • "_amount" as 1,000
    • "_deposit" as 1,000
    • "_hasFallbackToggled" as true
  11. The _createSettlement internal function in RootBridgeAgent will be executed and it will do the following:

    • Retrieve the local address of the global hUSDC token and save it in variable "localAddress"
    • Retrieve the underlying address of the "localAddress", which will return the address of the underlying USDC token
    • Call the internal function _updateStateOnBridgeOut and supply the following parameters:
      • "depositor" as "msg.sender" which will be the address of "MulticallRootRouter" since it's the ones who initiated the request/call
      • "_globalAddress" as the global hUSDC token address
      • "_localAddress" as the avalanche local hToken address
      • "_underlyingAddress" as the underlying USDC token address
      • "_amount" will be 1,000
      • "_deposit" will be 1,000"
      • "_dstChainId" will be the chain id of the avalanche chain
  12. In the internal function _updateStateOnBridgeOut. The second conditional will be executed "_deposit > 0" (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L1156C3-L1156C3). Then it will reach out to RootPort asking it to burn the 1,000 global hUSDC tokens.

  13. Execution will return back to callOutAndBridge now that _createSettlement is done.

  14. Function callOutAndBridge (which is in RootBridgeAgent) will perform a call to the Avalanche branch bridge agent

  15. In BranchBridgeAgent, deposit flag 1 conditional will be triggered (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L616)

  16. BranchBridgeAgent calls the function executeWithSettlement in BranchBridgeAgentExecutor. (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgentExecutor.sol#L66).

  17. The settlement object will be parsed and it's values will be supplied to the function "clearToken" in "BranchBridgeAgent"

  18. clearToken in BranchBridgeAgent will be executed (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L485C14-L485C24) and It will call the internal function _clearToken.

  19. Internal function _clearToken will be executed and only the second conditional will be triggered. (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L913)

  20. The function "withdraw" in BranchPort will be executed (https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchPort.sol#L226)

  21. And finally, it will transfer the locked 1,000 underlying "USDC" tokens to the "recipient" which is the attacker

PoC Code

In "RootForkTest.t.sol" please import "forge-std/console.sol" and then add the following function. and run the following command: forge test --match-contract RootForkTest --match-test test_exploit_1 -vvv

    function test_exploit_1() public {

        switchToLzChain(avaxChainId);

        address user = makeAddr("user"); // Create an address for the user
        address attacker = makeAddr("attacker"); // Create an address for the attacker

        address underlyingUSDC_addr;
        address local_hUSDC_addr;
        address global_hUSDC_addr;

        // Get some gas for the user, attacker and this test file.
        vm.deal(user, 5000 ether);
        vm.deal(address(this), 5000 ether);
        vm.deal(attacker, 5000 ether);

        // -------------- Create Mock usdc Token and mint 10,000 usdc tokens to user --------------

        uint256 userMintAmount = 10000 ether;

        MockERC20 usdc = new MockERC20("usdc", "usdc", 18);

        underlyingUSDC_addr = address(usdc);

        usdc.mint(user, userMintAmount);

        uint256 balanceOfUserAfterMint = MockERC20(underlyingUSDC_addr).balanceOf(user);

        require(balanceOfUserAfterMint == userMintAmount);

        // -------------- Add Mock USDC Underlying token to the system --------------

        avaxCoreRouter.addLocalToken{value: 10 ether}(underlyingUSDC_addr, GasParams(2_000_000, 0));

        switchToLzChain(rootChainId);

        vm.startPrank(address(rootPort));

        local_hUSDC_addr = RootPort(rootPort).getLocalTokenFromUnderlying(address(usdc), avaxChainId);
        global_hUSDC_addr = RootPort(rootPort).getGlobalTokenFromLocal(local_hUSDC_addr, avaxChainId);

        vm.stopPrank();

        // -------------- User Deposits 10,000 USDC tokens into the protocol --------------

        switchToLzChain(avaxChainId);

        DepositInput memory depositInput = DepositInput({
            token: underlyingUSDC_addr,
            hToken: local_hUSDC_addr,
            amount: balanceOfUserAfterMint,
            deposit: balanceOfUserAfterMint
        });

        vm.startPrank(user);

        usdc.approve(address(avaxMulticallRouter), balanceOfUserAfterMint);

        avaxMulticallRouter.callOutAndBridge{value: 100 ether}("", depositInput, GasParams(800_000, 0.1 ether));

        vm.stopPrank();

        // -------------- Attack execution steps to steal the 10,000 USDC from the protocol --------------

        switchToLzChain(avaxChainId);

        Multicall2.Call[] memory calls = new Multicall2.Call[](1);

        // Mock Omnichain dApp call
        calls[0] = Multicall2.Call({target: arbitrumGlobalToken, callData: abi.encodeWithSelector(bytes4(keccak256(bytes("distro()"))))});

        OutputParams memory outputParams = OutputParams({
            recipient: attacker,
            outputToken: global_hUSDC_addr,
            amountOut: balanceOfUserAfterMint,
            depositOut: balanceOfUserAfterMint
        });

        bytes memory packedData = abi.encodePacked(
            bytes1(0x02), 
            abi.encode(calls, outputParams, avaxChainId, GasParams(1_500_000, 6 ether))
        );

        vm.prank(attacker);
        vm.deal(attacker, 500 ether);

        avaxMulticallBridgeAgent.callOut{value: 500 ether}(
            payable(address(avaxMulticallBridgeAgent)), packedData, GasParams(1_500_000, 0.1 ether)
        );
        switchToLzChain(rootChainId);

        switchToLzChain(avaxChainId);

        uint256 attackerUSDCBalance = MockERC20(underlyingUSDC_addr).balanceOf(attacker);
        uint256 userUSDCBalance = MockERC20(underlyingUSDC_addr).balanceOf(user);

        if (attackerUSDCBalance == balanceOfUserAfterMint && userUSDCBalance == 0) {
            console.log("Attacker stole user funds successfully!"); // Attack successful.
        }

    }

Tools Used

VSCode, Foundry

Recommended Mitigation Steps

There needs to be a way to know how much global hTokens belong to each user on root level, for example a mapping. And when executing a settlement, the system should check if in fact the requester owns the amount of underlying/global tokens he did request to get back.

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #685

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

alcueca commented 1 year ago

The Router is not expected to hold funds, and callers of unsigned functions should know that. They are minted in the Router to be immediately used. If they make an error and leave their tokens in the Router, then it not expected that they will be protected.

c4-judge commented 1 year ago

alcueca marked the issue as grade-c