howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

In the _transferOutAndCallV5 function, the original intention is to send ETH directly to the recipient if the swap is unsuccessful. This is stated in the comments. However, the code incorrectly sends ETH to the target, and then the TransferOutAndCallV5 event is triggered to indicate successful execution. This will cause users to lose funds.

Proof of Concept

First, perform the exchange in the _transferOutAndCallV5 function:

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

Then if it is unsuccessful, the ETH is sent to the target, which causes a loss of funds in the recipient:

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

Tools Used

manual

Recommended Mitigation Steps

-       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

Other

c4-judge commented 4 months ago

trust1995 marked the issue as unsatisfactory: Invalid