[H-01] Deadline check is not effective

[ddimitrov22]

Impact: High, because the transaction might be left hanging in the mempool and be executed way later than the user wanted at a possibly worse price

Likelihood: Medium, because there is a great chance that the user won't adjust the gas price to be lucrative for the validators to include its transaction fast

The deadline parameter in swapExactTokensForTokensSupportingFeeOnTransferTokens, swapExactETHForTokensSupportingFeeOnTransferTokens, and swapExactTokensForETHSupportingFeeOnTransferTokens() which are called in the convert methods inside ExchangeAgent.sol is hardcoded to block.timestamp.

Example in _convertTokenForETH:

    function _convertTokenForETH(
        address _dexAddress,
        address _token,
        uint256 _convertAmount,
        uint256 _desiredAmount
    ) private returns (uint256) {
        ...
        if (IUniswapFactory(_factory).getPair(_token, WETH) != address(0)) {
            address[] memory path = new address[](2);
            path[0] = _token;
            path[1] = WETH;
            _dexRouter.swapExactTokensForETHSupportingFeeOnTransferTokens(
                _convertAmount,
                _desiredAmount,
                path,
                msg.sender,
                block.timestamp //@audit deadline
            );
        }
        ...
    }

The swapExactTokensForETHSupportingFeeOnTransferTokens in UniswapV2Router02 contract:

    function swapExactTokensForETHSupportingFeeOnTransferTokens(
        ...
        uint deadline
    )
        external
        virtual
        override
        ensure(deadline)
{

The deadline parameter enforces a time limit by which the transaction must be executed otherwise it will revert.

Let's take a look at the ensure modifier that is present in the functions you are calling in UniswapV2Router02 contract:

modifier ensure(uint deadline) {
        require(deadline >= block.timestamp, 'UniswapV2Router: EXPIRED');
        _;
    }

Now when the deadline is hardcoded as block.timestamp, the transaction will not revert because the require statement will always be fulfilled by block.timestamp == block.timestamp.

If a user chooses a transaction fee that is too low for miners to be interested in including the transaction in a block, the transaction stays pending in the mempool for extended periods, which could be hours, days, weeks, or even longer.

This could lead to users getting a worse price because a validator can just hold onto the transaction.

Recommendations

Protocols should let users who interact with AMMs set expiration deadlines. Without this, there's a risk of a serious loss of funds for anyone starting a swap.

Use a user-supplied deadline instead of block.timestamp.

[wankhede04]

fixed at commit ab6376e