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

6 stars 3 forks source link

If calling `swapOutV5` would fail on target contract this could lead to loss of funds #88

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

Inside transferOutAndCallV5 there's a call to target contract with the intention to swap ETH/token to some other token, in case of ETH if swap would fail - router contract would try to send ETH to target contract, if contract has receive() function this would result in loss of funds.

Proof of Concept

Example:

  1. Alice calls `transferOutAndCallV5' with the intention of converting ETH to USDT and sending it to Bob.
  2. Router calls swapOutV5 function on destination contract.
  3. Swap fails due to slippage.
  4. Router sends all of Alice's ETH to the target contract. If contract has receive function implementation - Alice would lose all ETH she sent.

Tools Used

Manual review.

Recommended Mitigation Steps

Revert in case of unsuccessful swap or return ETH to recipient/msg.sender instead.

Assessed type

ETH-Transfer

c4-judge commented 4 months ago

trust1995 marked the issue as unsatisfactory: Invalid

c4-judge commented 4 months ago

trust1995 marked the issue as unsatisfactory: Out of scope