UniswapV2Handler uses block.timestamp as deadline parameter for swaps which could attract mev.

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x0567150f7518ad476c5178698309d7ebecf26449ce5d5fcb19f602f767224c73 Severity: medium

Description: Description\ UniswapV2Handler uses block.timestamp as deadline parameter for swaps which could attract mev.

Attack Scenario\ Passing block.timestamp as the expiry/deadline of an operation does not mean "require immediate execution" - it means "whatever block this transaction appears in, I'm comfortable with that block's timestamp". Providing this value means that a malicious miner can hold the transaction for as long as they like (think the flashbots mempool for bundling transactions), which may be until they are able to cause the transaction to incur the maximum amount of slippage allowed by the slippage parameter, or until conditions become unfavorable enough that other orders, e.g. liquidations, are triggered. Timestamps should be chosen off-chain, and should be specified by the caller to avoid unnecessary MEV.

Attachments

1. Proof of Concept (PoC) File

   function swapTokensToETH(
   uint256 _swapAmount,
   uint256 _slippage,
   address _t,
   address _to,
   bool isEnabled
   ) external returns (uint256 swapResult) {
   TransferHelper.safeApprove(_t, address(uniSwapRouter), _swapAmount);
   uint256 internalSlippage = isEnabled
     ? getSlippage(_swapAmount, _slippage, getPathForToken(_t))
     : 1;
   swapResult = uniSwapRouter.swapExactTokensForETH(
     _swapAmount,
     internalSlippage,
     getPathForToken(_t),
     _to,
     block.timestamp
   )[1];
   }

Above function uses block.timestamp as deadline parameter which is not recommended.

Look at these past issue here

2. Revised Code File (Optional) Caller should pass in a desired deadline which should be passed to the swap as the deadline parameter.

[langnavina97]

This issue is out of scope as the front-end-helper folder is excluded.