evan - triggerEndEpoch can be called on null epochs

[sherlock-admin]

evan

high

triggerEndEpoch can be called on null epochs

Summary

When 1 of the 2 vaults have no underlying asset at the end of the epoch, triggerNullEpoch should be called instead of triggerEndEpoch (both are functions in the controller). However, this isn't enforced, so it's possible to call triggerEndEpoch when this happens.

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L181-L192
triggerEndEpoch sends the premium to the collateral vault at the end of the epoch.

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L232-L251
triggerNullEpoch detects whether 1 of the 2 vaults have no underlying assets and resolves them without any fund-sending.

However, triggerEndEpoch doesn't verify that both vaults have underlying assets, so it's possible to invoke it when triggerNullEpoch should called.

Impact

Suppose users have deposited into the premium vault, but no one has deposited in the collateral vault. At the end of the epoch, a malicious user calls triggerEndEpoch instead of triggerNullEpoch (frontrunning everyone else). The premium will be sent to the collateral vault but no one can receive them, so this fund is permanently locked in the contract.

This vulnerability essentially exploits the incomplete mitigation of this vulnerability from a previous audit, so it has the same impact.

Code Snippet

Add this test to EndToEndV2Test.t.sol, and run it by itself forge test -m testEndEpochWhenNull

function testEndEpochWhenNull() public {
        vm.startPrank(USER);

        vm.warp(begin - 1 days);

        MintableToken(UNDERLYING).approve(premium, DEPOSIT_AMOUNT);
        VaultV2(premium).deposit(epochId, DEPOSIT_AMOUNT, USER);

        // warp to epoch end
        vm.warp(end + 1 days);
        
        uint256 prev_balance = MintableToken(UNDERLYING).balanceOf(collateral);
        // we should really be triggering null epoch (you can verify that it works too)
        // controller.triggerNullEpoch(marketId, epochId);

        // trigger end epoch
        controller.triggerEndEpoch(marketId, epochId);
        uint256 cur_balance = MintableToken(UNDERLYING).balanceOf(collateral);

        // fund was sent to the collateral vault
        assertEq(cur_balance - prev_balance, VaultV2(collateral).claimTVL(epochId));
        assertTrue(VaultV2(collateral).claimTVL(epochId) > 0);
        // but no one can receive them
        assertEq(VaultV2(collateral).totalSupply(epochId), 0);
        vm.stopPrank();
    }

Tool used

Manual Review
Foundry

Recommendation

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L81-L86
Add this check to triggerEndEpoch as well.

Duplicate of #108