Lines of code

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

Vulnerability details

Impact & Proof of concept

The code in transferOut() function first checks if it is Eth transfer it tries to send it to the target, if not successful it will transfer Eth back to the vault. No matter what the choice was, the event eventually will be emitted the same way.

    emit TransferOut(msg.sender, to, asset, safeAmount, memo);

this will mislead the smartcontract_log_parser into thinking that a transfer was successful when it wasn't.

Tools Used

manual review

Recommended Mitigation Steps

Add one more parameter in transferOut event to indicate wether the transfer succeeded or not.

- event TransferOut(
    address indexed vault,
    address indexed to,
    address asset,
    uint amount,
    string memo
  );
+ event TransferOut(
    address indexed vault,
    address indexed to,
    address asset,
    uint amount,
    string memo,
    bool success
  );

NOTE: The same issue exists in:

_transferOutV5()
transferOutAndCall()
_transferOutAndCallV5()

Assessed type

Other

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