howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

Send the eth to the aggregator instead of the user 'recipient'. In this way, the aggregator will have the eth and the user will not, this is not the desired behavior.

Proof-of-concept

In the _transferOutAndCallV5() function, line 324, you send eth to the aggregator instead of the user.

bool sendSuccess = payable(aggregationPayload.target).send(msg.value);

Unlike in the transferOutAndCall() function, line 278 where you send the eth to the user when swapOutV5() function fails:

bool ethSuccess = payable(to).send(_safeAmount);

Tools Used

Manual review

Recommended Mitigation Steps

Change the line 324:

-bool sendSuccess = payable(aggregationPayload.target).send(msg.value);
+bool sendSuccess = payable(aggregationPayload.recipient).send(msg.value);

Assessed type

Context

c4-judge commented 4 months ago

trust1995 marked the issue as unsatisfactory: Invalid