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

1 stars 0 forks source link

Hardcoding the expiration date in _routerDeposit( ) to type(uint).max can lead to halting fund transfer to the new router #238

Closed c4-bot-5 closed 4 months ago

c4-bot-5 commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The process of migrating funds from the old router to the new one is handled with _routerDeposit() internal function.

function _routerDeposit(
    address _router,
    address _vault,
    address _asset,
    uint _amount,
    string memory _memo
  ) internal {
    _vaultAllowance[msg.sender][_asset] -= _amount;
    safeApprove(_asset, _router, _amount);

    iROUTER(_router).depositWithExpiry(
      _vault,
      _asset,
      _amount,
      _memo,
      type(uint).max
    ); // Transfer by depositing
  }

As can be seen, the function is calling depositWithExpiry() passing the required parameters, yet the expiration parameter is set to its maximum value opening the door to any malicious validator to manipulate it to gain profit.

Tools Used

manual review

Recommended Mitigation Steps

the expiration should not be hardcoded. It is better to be user defined with proper checks.

Assessed type

MEV