`_transferOutAndCallV5` transfers gas assets to a wrong address when the swap fails, resulting in the loss of user funds

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Vulnerability details

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

  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

      snip...

When the transferOutAndCallV5 or batchTransferOutV5 function is called by a THORChain vault, the _transferOutAndCallV5 internal function is triggered.

_transferOutAndCallV5 first tries to call the aggregator to execute a swap. If the swap fails (e.g. when the actual amountOut is less than the specified amountOutMin), the control flow goes to the if (!swapOutSuccess) {} path.

If can't swap, just send the recipient the gas asset

As indicated by the comment, in the above scenario, the gas assets should be sent directly to the recipient. However, instead of sending them to aggregationPayload.recipient, the code sends them to aggregationPayload.target, which is the address of the aggregator.

As a result, the asset are sent to the wrong address, preventing the recipient from receiving their rightful funds.

Impact

Loss of funds, recipient not receiving the rightful funds.

Tools Used

Manual review.

Recommended Mitigation Steps

Fixing the wrong receiving address in _transferOutAndCallV5:

if (!swapOutSuccess) {
-         bool sendSuccess = payable(aggregationPayload.target).send(msg.value);
+         bool sendSuccess = payable(aggregationPayload.recipient).send(msg.value);

Assessed type

ETH-Transfer

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope