Vast Umber Walrus - Inability to shutdown/sunset a newly registered collection after previous shutdown

[sherlock-admin3]

Vast Umber Walrus

Medium

Inability to shutdown/sunset a newly registered collection after previous shutdown

Summary

The shutdownVotes parameter is not cleared after a collection shutdown is executed and claimed. This issue prevents a previously shut down collection from being eligible for a new shutdown/sunset process if it is re-registered.

Vulnerability Detail

When the CollectionShutdown contract executes and then the claim process occurs, it fails to update or clear the shutdownVotes parameter, which is used to track votes related to the shutdown process.

CollectionShutdown::claim()

File: CollectionShutdown.sol
285:     function claim(address _collection, address payable _claimant) public nonReentrant whenNotPaused {
286:         // Ensure our user has tokens to claim
287:         uint claimableVotes = shutdownVoters[_collection][_claimant];
288:         if (claimableVotes == 0) revert NoTokensAvailableToClaim();
289: 
290:         // Ensure that we have moved token IDs to the pool
291:         CollectionShutdownParams memory params = _collectionParams[_collection];
292:         if (params.sweeperPool == address(0)) revert ShutdownNotExecuted();
293: 
294:         // Ensure that all NFTs have sold from our Sudoswap pool
295:         if (!collectionLiquidationComplete(_collection)) revert NotAllTokensSold();
296: 
297:         // We can now delete our sweeper pool tokenIds
298:         if (params.sweeperPoolTokenIds.length != 0) {
299:             delete _collectionParams[_collection].sweeperPoolTokenIds;
300:         }
301: 
302:         // Burn the tokens from our supply
303:         params.collectionToken.burn(claimableVotes);
304: 
305:         // Set our available tokens to claim to zero
306:         delete shutdownVoters[_collection][_claimant];
307: 
308:         // Get the number of votes from the claimant and the total supply and determine from that the percentage
309:         // of the available funds that they are able to claim.
310:         uint amount = params.availableClaim * claimableVotes / (params.quorumVotes * ONE_HUNDRED_PERCENT / SHUTDOWN_QUORUM_PERCENT);
311:         (bool sent,) = _claimant.call{value: amount}('');
312:         if (!sent) revert FailedToClaim();
313: 
314:         emit CollectionShutdownClaim(_collection, _claimant, claimableVotes, amount);
315:     }

If a collection that was previously shut down is re-registered with the Locker, it will not be eligible for a new shutdown/sunset process because the shutdownVotes from the previous shutdown remain unchanged.

CollectionShutdown::start()

File: CollectionShutdown.sol
135:     function start(address _collection) public whenNotPaused {
136:         // Confirm that this collection is not prevented from being shutdown
137:         if (shutdownPrevented[_collection]) revert ShutdownPrevented();
138: 
139:         // Ensure that a shutdown process is not already actioned
140:         CollectionShutdownParams memory params = _collectionParams[_collection];
141:@>       if (params.shutdownVotes != 0) revert ShutdownProcessAlreadyStarted();
---
157:     }

Impact

Prevents the CollectionShutdown contract from correctly processing a new shutdown/sunset request for a collection that has been previously shut down. This results in the collection being unable to be shut down again, even if it is re-registered.

Code Snippet

CollectionShutdown::claim()

File: CollectionShutdown.sol
285:     function claim(address _collection, address payable _claimant) public nonReentrant whenNotPaused {
286:         // Ensure our user has tokens to claim
287:         uint claimableVotes = shutdownVoters[_collection][_claimant];
288:         if (claimableVotes == 0) revert NoTokensAvailableToClaim();
289: 
290:         // Ensure that we have moved token IDs to the pool
291:         CollectionShutdownParams memory params = _collectionParams[_collection];
292:         if (params.sweeperPool == address(0)) revert ShutdownNotExecuted();
293: 
294:         // Ensure that all NFTs have sold from our Sudoswap pool
295:         if (!collectionLiquidationComplete(_collection)) revert NotAllTokensSold();
296: 
297:         // We can now delete our sweeper pool tokenIds
298:         if (params.sweeperPoolTokenIds.length != 0) {
299:             delete _collectionParams[_collection].sweeperPoolTokenIds;
300:         }
301: 
302:         // Burn the tokens from our supply
303:         params.collectionToken.burn(claimableVotes);
304: 
305:         // Set our available tokens to claim to zero
306:         delete shutdownVoters[_collection][_claimant];
307: 
308:         // Get the number of votes from the claimant and the total supply and determine from that the percentage
309:         // of the available funds that they are able to claim.
310:         uint amount = params.availableClaim * claimableVotes / (params.quorumVotes * ONE_HUNDRED_PERCENT / SHUTDOWN_QUORUM_PERCENT);
311:         (bool sent,) = _claimant.call{value: amount}('');
312:         if (!sent) revert FailedToClaim();
313: 
314:         emit CollectionShutdownClaim(_collection, _claimant, claimableVotes, amount);
315:     }

CollectionShutdown::start()

File: CollectionShutdown.sol
135:     function start(address _collection) public whenNotPaused {
136:         // Confirm that this collection is not prevented from being shutdown
137:         if (shutdownPrevented[_collection]) revert ShutdownPrevented();
138: 
139:         // Ensure that a shutdown process is not already actioned
140:         CollectionShutdownParams memory params = _collectionParams[_collection];
141:@>       if (params.shutdownVotes != 0) revert ShutdownProcessAlreadyStarted();
---
157:     }

Tool used

Manual Review

Recommendation

Update the params.shutdownVoters when claiming to reduce global votes.

function claim(address _collection, address payable _claimant) public nonReentrant whenNotPaused {
    // Ensure our user has tokens to claim
    uint claimableVotes = shutdownVoters[_collection][_claimant];
    if (claimableVotes == 0) revert NoTokensAvailableToClaim();
---

+   params.shutdownVotes -= claimableVotes;

    emit CollectionShutdownClaim(_collection, _claimant, claimableVotes, amount);
}

function reclaimVote(address _collection) public whenNotPaused {
    // If the quorum has passed, then we can no longer reclaim as we are pending
    // an execution.
    CollectionShutdownParams storage params = _collectionParams[_collection];
    if (params.canExecute) revert ShutdownQuorumHasPassed();
+   if (params.sweeperPool != address(0)) revert ShutdownExecuted(); // prevent from reclaim after executed
---
}

However, this approach raises another concern: the params.shutdownVoters represents votes from users, which could potentially lead to an imbalance in the total value relative to the supply.

In a scenario where all claimers reclaim their votes, params.shutdownVoters would revert to zero, enabling a new shutdown process. However, users who hold collection tokens but haven’t voted (or claimed via CollectionShutdown::voteAndClaim()) will be affected.

If the collection is re-registered and undergoes another shutdown, those non-voting users could lose the ability to claim rewards, as new tokens might be minted for the re-registered collection, leaving their tokens unaccounted for.

This concern should be evaluated based on the specific business logic in place.