Breeje - Setting Deadline for `swapExactTokensForTokens` as `type(uint256).max` allows Exploits

[sherlock-admin]

Breeje

medium

Setting Deadline for swapExactTokensForTokens as type(uint256).max allows Exploits

Summary

While Closing Position Farm in AuraSpell, convexSpell and curveSpell, there is a use of Uniswap Router which uses swapExactTokensForTokens method call to Swap rewards tokens to debt token. But the value of deadline is set to be type(uint256).max which allows transaction to maliciously executed at a later stage in an unfavorable condition leading to exploits.

Vulnerability Detail

In closePositionFarm method of all 3 spell contracts, the deadline check is set to type(uint256).max which means the miner can manipulate and execute the transaction wherever it is the most favorable for him/her. Firstly, there is no Slippage check present in the contract, and even if after mitigation, it is added, This attack vector allows the transaction to happen after a long time in future which can to bad swap as a valid slippage value will also be stale as it is passed considering today's scenario.

Here's the scenario of how this can happen:

1. There is a call made to closePositionFarm. The transaction can be pending in mempool for a long time and the trading activity is very time senstive.
2. With deadline check set to type(uint256).max, the trade transaction can be executed in a long time after the caller submit the transaction, at that time, the trade can be done in a sub-optimal price, which harms caller's position.
3. In this scenario, even slippage parameter won't completely rescue caller. Let's say the slippage is set to 1% at today's price results in amountOutMin value of x. But in future, that x value can potentially lead to 20% slippage as the price has gone up for that token.
4. Miner can sandwich attack after long time when it is favorable for him/her to get the maximum slippage value out of this transaction at Alice's expense.

This is the reason having a valid deadline check is very important as it ensures that the transaction will be executed on time and the expired transaction will reverts which prevent loss of funds. Also Given both issues having different attack vectors: Slippage Check protects against MEV Bots sandwiching their transaction while deadline protects against executing the transaction till the time slippage value is valid, Reporting it as different medium issue than slippage one.

Impact

Loss of Funds.

Code Snippet

File: AuraSpell.sol

  swapRouter.swapExactTokensForTokens(
      rewards,
      0,
      swapPath,
      address(this),
      type(uint256).max
  );

Link to Code

File: ConvexSpell.sol

  swapRouter.swapExactTokensForTokens(
      rewards,
      0,
      swapPath,
      address(this),
      type(uint256).max
  );

Link to Code

File: CurveSpell.sol

  swapRouter.swapExactTokensForTokens(
      rewards,
      0,
      swapPath,
      address(this),
      type(uint256).max
  );

Link to Code

Tool used

Manual Review

Recommendation

Add a valid Deadline.

Duplicate of #145