search

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

6 stars 3 forks source link

Unsuccessful swap could lead to loss of ERC20 tokens #87

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

Impact

Inside transferOutAndCallV5 there is a call to the target contract with the intention to swap ETH/token for another token, in the case of ERC20 if the swap fails - router won't revert the transaction and because router has already sent tokens to the target contract - this would result in a loss of funds.

Proof of Concept

There is no check for the _dexAggSuccess value, and the comment 'if the aggregator fails, the transaction should not revert', this means that unsuccessful swap would result in simply sending your tokens to the aggregationPayload.target contract instead of doing a swap.

Tools Used

Manual review.

Recommended Mitigation Steps

Add a swap execution check and revert if it fails.

Assessed type

ERC20

c4-judge commented 3 months ago

trust1995 marked the issue as unsatisfactory: Invalid