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

6 stars 3 forks source link

`THORChain_Router::batchTransferOutAndCallV5` function uses `msg.value` in a loop which makes the function unusable for batch transfers of ETH #77

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#L309-L311

Vulnerability details

Impact

This error makes the THORChain_Router::batchTransferOutAndCallV5 function unusable for batch swapping ETH, because the full ETH value will be forwarded and depleted with the first transaction causing a revert. This breaks an important functionality for more efficient swaps of large amounts of assets.

Tools Used

Slither

Recommended Mitigation Steps

The fromAmount property on the THORChain_Router::TransferOutAndCallData should be used as the value to send when calling the swapOutV5 function and the THORChain_Router::batchTransferOutAndCallV5 should keep track whether the msg.value has been depleted on each loop iteration and revert if so.

Assessed type

ETH-Transfer

c4-judge commented 4 months ago

trust1995 marked the issue as satisfactory