ETH is transferred to the wrong address in _transferOutAndCallV5

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/main/chain/ethereum/contracts/THORChain_Router.sol#L324

Vulnerability details

Impact

The _transferOutAndCallV5 function transfers ETH tokens to the wrong address if the aggregator fails.

Proof of Concept

When the _transferOutAndCallV5 function is called and the aggregator fails to execute the swapOutV5 transaction, the ETH value should be sent to the swap recipient directly. However instead of the recipient, ETH is sent to the aggregator contract:

  function _transferOutAndCallV5(
    TransferOutAndCallData calldata aggregationPayload
  ) private {
    if (aggregationPayload.fromAsset == address(0)) {
      // call swapOutV5 with ether
      (bool swapOutSuccess, ) = aggregationPayload.target.call{
        value: msg.value
      }(
        abi.encodeWithSignature(
          "swapOutV5(address,uint256,address,address,uint256,bytes,string)",
          aggregationPayload.fromAsset,
          aggregationPayload.fromAmount,
          aggregationPayload.toAsset,
          aggregationPayload.recipient,
          aggregationPayload.amountOutMin,
          aggregationPayload.payload,
          aggregationPayload.originAddress
        )
      );
      if (!swapOutSuccess) {
>>      bool sendSuccess = payable(aggregationPayload.target).send(msg.value); // If can't swap, just send the recipient the gas asset

Notice the correct implementation in the transferOutAndCall function:

  function transferOutAndCall(
    address payable aggregator,
    address finalToken,
    address to,
    uint256 amountOutMin,
    string memory memo
  ) public payable nonReentrant {
    uint256 _safeAmount = msg.value;
    (bool erc20Success, ) = aggregator.call{value: _safeAmount}(
      abi.encodeWithSignature(
        "swapOut(address,address,uint256)",
        finalToken,
        to,
        amountOutMin
      )
    );
    if (!erc20Success) {
>>    bool ethSuccess = payable(to).send(_safeAmount); // If can't swap, just send the recipient the ETH

Tools Used

Manual review

Recommended Mitigation Steps

      if (!swapOutSuccess) {
-       bool sendSuccess = payable(aggregationPayload.target).send(msg.value); // If can't swap, just send the recipient the gas asset
+       bool sendSuccess = payable(aggregationPayload.recipient).send(msg.value); // If can't swap, just send the recipient the gas asset

Assessed type

ETH-Transfer

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope