Malicious user can flood the monitoring queue

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

As stated in the readme, there will be monitoring of the events that is done off-chain, the problem is that the depositWithExpiry() in THORChain_Router.sol can receive 0 as amount and it will still emit an event. A malicious user can just spam events like this which will result in flooding the monitoring queue

Proof of Concept

This is the implementation of the _deposit() https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/ethereum/contracts/THORChain_Router.sol#L143

 function _deposit(
    address payable vault,
    address asset,
    uint amount,
    string memory memo
  ) private nonReentrant {
    //@audit flood with 0s
    uint safeAmount;
    if (asset == address(0)) {
      safeAmount = msg.value;
      bool success = vault.send(safeAmount);
      require(success);
    } else {
      require(msg.value == 0, "unexpected eth"); // protect user from accidentally locking up eth
      safeAmount = safeTransferFrom(asset, amount); // Transfer asset
      _vaultAllowance[vault][asset] += safeAmount; // Credit to chosen vault
    }
    emit Deposit(vault, asset, safeAmount, memo);
  }

As we can see there are no checks for 0 amount.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add require statement in the depositWithExpiry() that prevents amount 0 to be passed.

Assessed type

Error