setParam() cooldown can cause loss of fund to liquidity providers

Summary

Cooldown changed by admin intentionally or unintentionally during the active period can cause liquidity providers to be unable to withdraw from liquidity pool after maturity without slippages.

Vulnerability Detail

A malicious party can roll the AutoRoller immediately if cooldown is set to zero or an arbitrary small value just before sponsor settles the series which triggers cooldown, causing a loss of fund to liquidity providers who are not aware of the change in cooldown value. else if (lastSettle + cooldown > block.timestamp) will always be false when cooldown is set to zero.

Impact

Loss of fund to liquidity providers as they have to exit the liquidity with slippages.

Code Snippet

AutoRoller.sol#L154-L167


    function roll() external {
        if (maturity != MATURITY_NOT_SET) revert RollWindowNotOpen();

        if (lastSettle == 0) {
            // If this is the first roll, lock some shares in by minting them for the zero address.
            // This prevents the contract from reaching an empty state during future active periods.
            deposit(firstDeposit, address(0));
        } else if (lastSettle + cooldown > block.timestamp) {
            revert RollWindowNotOpen();
        }

        lastRoller = msg.sender;
        adapter.openSponsorWindow();
    }

AutoRoller.sol#L824-L831


    function setParam(bytes32 what, uint256 data) external {
        require(msg.sender == owner);
        if (what == "MAX_RATE") maxRate = data;
        else if (what == "TARGET_DURATION") targetDuration = data;
        else if (what == "COOLDOWN") {
            require(lastSettle == 0 || maturity != MATURITY_NOT_SET); // Can't update cooldown during cooldown period.
            cooldown = data;
        }

Tool used

Manual Review

Recommendation

set a lower bound to the cooldown of the autoroller (ie: 1 hour) so that liquidity providers can have certainty that there is a time period for them to withdraw their liquidity without slippages.

   require(data >= 1 hours);
   cooldown = data;

however, if a cooldown of 0 is required due to some edge cases, we can prevent liquidity provider from getting caught off guard by last minute changes to the cooldown period by disallowing admin from changing cooldown just right before maturity (ie: 1 hour) .

   require( block.timestamp - maturity >= 1 hours);

sherlock-audit / 2022-11-sense-judging

koxuan - setParam() cooldown can cause loss of fund to liquidity providers #17