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

1 stars 0 forks source link

ETH is sent to the wrong address #227

Closed c4-bot-7 closed 4 months ago

c4-bot-7 commented 4 months ago

Lines of code

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

Vulnerability details

Impact

Inside the _transferOutAndCallV5(), if the function enters the first branch and the swapOutV5 call fails, on line 324, a send() to the aggregationPayload.target address will be performed.

However, this address is the aggregator called in the place. The aggregator has a receive() so the call will not fail and the funds will be stuck inside the aggregator. If another user/ bot observes this and is faster than the user they can call themselves swapOutV5 aggregator contract, which received those new funds, and get a free swap(minus the gas fees)

Proof of Concept

  1. User calls transferOutAndCallV5() in order to make a swap
  2. The tx reverts inside the aggregator for some reason (the actual amount received after the swap is < amountOutMin param)
  3. A call is made on line 324 to the aggregationPayload.target address. This is however not the intended recipient.
  4. Another user or a bot observes this transfer of ETH and can easily call swapOutV5 inside the aggregator and make a free swap

Tools Used

Mannual review

Recommended Mitigation Steps

Replace payable(aggregationPayload.target) with payable(aggregationPayload.recipient)

Assessed type

ETH-Transfer