Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/ethereum/contracts/THORChain_Router.sol#L307-L328

Vulnerability details

Impact

The batchTransferOutAndCallV5 function in the THORChain_Router contract fails when trying to process multiple TransferOutAndCallData payloads due to an "out of funds" error. This occurs because the function does not correctly handle the total Ether amount required for all payloads, leading to a situation where the transaction reverts even if sufficient funds were initially provided. This prevents the function from executing the batch transfer and can disrupt operations that rely on batch processing.

Proof of Concept

The following proof of concept demonstrates how the batchTransferOutAndCallV5 function fails when trying to process two payloads with a combined Ether amount that matches the initial balance but causes an "out of funds" error.

Test Case (Foundry)

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.22;

import {Test, console} from "forge-std/Test.sol";
import {THORChain_Router} from "../contracts/THORChain_Router.sol";
import {THORChain_Aggregator} from "../contracts/THORChain_Aggregator.sol";
import {SushiRouterSmol} from "../contracts/sushiswap/SushiRouterSmol.sol";
import {WETH} from "../contracts/sushiswap/WETH.sol";
import {ERC20Token} from "../contracts/Token.sol";

contract RouterPOC is Test {
    THORChain_Router public router;
    THORChain_Aggregator public aggregator;

    SushiRouterSmol public sushiRouter;
    WETH public weth;
    ERC20Token public token;

    address public vault;

    function setUp() public {
        router = new THORChain_Router();
        weth = new WETH();
        sushiRouter = new SushiRouterSmol(address(weth));
        aggregator = new THORChain_Aggregator(address(weth), address(sushiRouter));
        token = new ERC20Token();

        vault = makeAddr("Vault");
    }

    function testPOC_batchTransferOutAndCallV5_outOfFunds() public {
        uint256 allSwapOutAmounts = 10e18;
        deal(vault, allSwapOutAmounts);

        // setup a batchTransferOutAndCallV5 payloads
        THORChain_Router.TransferOutAndCallData[] memory aggrPayloads =
            new THORChain_Router.TransferOutAndCallData[](2);
        aggrPayloads[0] = THORChain_Router.TransferOutAndCallData(
            payable(address(aggregator)),
            address(0), // native token
            allSwapOutAmounts/2, // half of the value
            address(token),
            payable(makeAddr("recipient1")),
            0,
            "",
            bytes(""),
            ""
        );
        aggrPayloads[1] = THORChain_Router.TransferOutAndCallData(
            payable(address(aggregator)),
            address(0), // native token
            allSwapOutAmounts/2, // half of the value
            address(token),
            payable(makeAddr("recipient2")),
            0,
            "",
            bytes(""),
            ""
        );

        vm.prank(vault);
        vm.expectRevert(); // reverts because of "outOfFunds"
        router.batchTransferOutAndCallV5{value: allSwapOutAmounts}(aggrPayloads);

    }
}

Tools Used

Foundry

Recommended Mitigation Steps

To mitigate this issue, the batchTransferOutAndCallV5 function should use the aggregationPayload.fromAmount rather than msg.value in the underlying _transferOutAndCallV5 function. Additionally, any excess Ether sent in the transaction should be returned to the sender instead of being left in the router contract.

Assessed type

Other

code-423n4 / 2024-06-thorchain-findings

`batchTransferOutAndCallV5` Fails Due to Out of Funds #12