Popcorn contest - Nyx's results

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L539-L546 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L629-L636

Vulnerability details

Impact

Vault owner can change fee parameters before quitPeriod, and users may have unexpected results.

Proof of Concept

uint256 public quitPeriod = 3 days;

Normally quitPeriod is set to 3 days.

function setQuitPeriod(uint256 _quitPeriod) external onlyOwner {
        if (_quitPeriod < 1 days || _quitPeriod > 7 days)
            revert InvalidQuitPeriod();

        quitPeriod = _quitPeriod;

        emit QuitPeriodSet(quitPeriod);
    }

When vault owner proposeFees, vault owner needs to wait three days to change fees. But after waiting for one-day vault owner can setQuitPeriod to 1 day.

function changeFees() external {
        if (block.timestamp < proposedFeeTime + quitPeriod) 
            revert NotPassedQuitPeriod(quitPeriod);

        emit ChangedFees(fees, proposedFees);
        fees = proposedFees;
    }

After changing quitPeriod to 1 day, Vault owners can changeFees() whenever they want.

Tools Used

Manual Review

Recommended Mitigation Steps