4b - Deadline cannot be block.timestamp in VaultLib::_redeemCtDsAndSellExcessCt

[sherlock-admin3]

4b

Medium

Deadline cannot be block.timestamp in VaultLib::_redeemCtDsAndSellExcessCt

Summary

on line 345 of VaultLib::_redeemCtDsAndSellExcessCt, the function swapExactTokensForTokens() is called but the deadline parameter of the function is initialised to block.timestamp which offers no protection

Root Cause

on line 345 the deadline param of the swapExactTokensForTokens is being initialised to block.timestamp

Internal pre-conditions

whenever user calls redeemEarlyLv in the vault contract.

External pre-conditions

No external pre-con

Attack Path

1. When user calls redeemEarlyLv
2. Then it makes a call to the vaultlib redeemEarly using the current state storage
3. A call is made to _liquidateLpPartial
4. then we try to _redeemCtDsAndSellExcessCt
5. in _redeemCtDsAndSellExcessCt a call is made to ammRouter.swapExactTokensForTokens.
6. which makes an external call to uniswaps router v2 with the provided data

Impact

There is no real protection or deadline

PoC

looking into the function the problem is from this line
ra += ammRouter.swapExactTokensForTokens(ctSellAmount, 0, path, address(this),block.timestamp)[1];

which intend makes a swapExactTokensForTokens call to an amm router which is uniswapV2's router02, which looks like this;

    function swapExactTokensForTokens(
        uint amountIn,
        uint amountOutMin,
        address[] calldata path,
        address to,
        uint deadline
    ) external virtual override ensure(deadline) returns (uint[] memory amounts) {

so as we can see, in the function declaration there is an ensure modifier which makes sure the deadline has not passed.

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

so evaluating when you have a deadline of block.timestamp, we can confirm deadline will never be reached, because block.timestamp == block.timestamp always.

Mitigation

Add a real deadline either a user passed in timestamp or any other implementation

Duplicate of #66