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
User calls transferOutAndCallV5() in order to make a swap
The tx reverts inside the aggregator for some reason (the actual amount received after the swap is < amountOutMin param)
A call is made on line 324 to the aggregationPayload.target address. This is however not the intended recipient.
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)
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 theswapOutV5
call fails, on line 324, asend()
to theaggregationPayload.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 themselvesswapOutV5
aggregator contract, which received those new funds, and get a free swap(minus the gas fees)Proof of Concept
transferOutAndCallV5()
in order to make a swapamountOutMin
param)aggregationPayload.target
address. This is however not the intended recipient.swapOutV5
inside the aggregator and make a free swapTools Used
Mannual review
Recommended Mitigation Steps
Replace
payable(aggregationPayload.target)
withpayable(aggregationPayload.recipient)
Assessed type
ETH-Transfer