climber2002 - triggerNullEpoch could be frontrun by triggerEndEpoch

[sherlock-admin]

climber2002

high

triggerNullEpoch could be frontrun by triggerEndEpoch

Summary

In ControllerPeggedAssetV2, triggerNullEpoch could be frontrun by triggerEndEpoch

Vulnerability Detail

In triggerEndEpoch it doesn't validate that the totalAssets of premiumVault and collateralVault must both be positive. So let's suppose following scenario,

1. A market is created and user deposits into premiumVault but there is no deposits in collateralVault
2. The current timestamp has passed epochEnd, and then someone triggered triggerNullEpoch
3. A bot frontruns it and call triggerEndEpoch, the tokens in premiumVault will be transferred into collateralVault with fees transferred into treasury.

Actually triggerEndEpoch can always be triggered when epochEnd is reached

Impact

I think this is a high vulnerability as when this happens, since there is no deposits in collateralVault so all tokens transferred to it are locked and no one can withdraw it.

Code Snippet

I created following test in ControllerPeggedAssetV2Test. It proves that when there is no deposits in collateralVault, triggerEndEpoch can still be triggered and tokens will be transferred into collateralVault.

contract ControllerPeggedAssetV2Test is Helper {
    function testTriggerEndEpoch_noDepositsInCollateralVault() public {
        MintableToken(UNDERLYING).mint(USER, 10 ether);
        
        vm.startPrank(USER);
        MintableToken(UNDERLYING).approve(address(vault), 10 ether);
        vm.warp(begin - 1);
        vault.deposit(epochId, 10 ether, USER);
        vm.stopPrank();
        
        // Before triggerEndEpoch premiumVault has assets but collateralVault has NO assets
        assertEq(MintableToken(UNDERLYING).balanceOf(address(vault)), 10 ether);
        assertEq(MintableToken(UNDERLYING).balanceOf(address(counterpartyVault)), 0);
        assertEq(counterpartyVault.totalAssets(epochId), 0);
        
        vm.warp(end + 1);
        controller.triggerEndEpoch(marketId, epochId);
        
         // After triggerEndEpoch tokens are all transferred to collateralVault and treasury
        assertEq(MintableToken(UNDERLYING).balanceOf(address(vault)), 0);
        assertEq(MintableToken(UNDERLYING).balanceOf(address(counterpartyVault)) > 0, true);
    }
}

Tool used

Manual Review

Recommendation

In triggerEndEpoch add following validation

        if (
            premiumVault.totalAssets(_epochId) == 0 ||
            collateralVault.totalAssets(_epochId) == 0
        ) {
            revert VaultZeroTVL();
        }

Duplicate of #108