Unable to reclaim votes after collection shutdown cancellation

Summary

When a collection shutdown is canceled, the previous voter is unable to reclaim their vote due to the cancellation logic deleting the entire _collectionParams mapping. This results in inconsistent values and potential issues when the token returns to a shutdown state.

Vulnerability Detail

The CollectionShutdown::cancel() function deletes the _collectionParams mapping for a collection, which can lead to several problems:

CollectionShutdown::cancel()

File: CollectionShutdown.sol
390:     function cancel(address _collection) public whenNotPaused {
391:         // Ensure that the vote count has reached quorum
392:         CollectionShutdownParams memory params = _collectionParams[_collection];
393:         if (!params.canExecute) revert ShutdownNotReachedQuorum();
394: 
395:         // Check if the total supply has surpassed an amount of the initial required
396:         // total supply. This would indicate that a collection has grown since the
397:         // initial shutdown was triggered and could result in an unsuspected liquidation.
398:         if (params.collectionToken.totalSupply() <= MAX_SHUTDOWN_TOKENS * 10 ** locker.collectionToken(_collection).denomination()) {
399:             revert InsufficientTotalSupplyToCancel();
400:         }
401: 
402:         // Remove our execution flag
403:         delete _collectionParams[_collection];
404:         emit CollectionShutdownCancelled(_collection);
405:     }

This issue can prevent users from reclaiming their votes due to an underflow error:

CollectionShutdown::reclaimVote()

File: CollectionShutdown.sol
356:     function reclaimVote(address _collection) public whenNotPaused {
---
368:         // We delete the votes that the user has attributed to the collection
369:@>       params.shutdownVotes -= uint96(userVotes);
---
377:     }

Moreover, this can lead to inconsistencies in the shutdown state of collections:

Assuming:

Before cancellation, totalSupply was 400 and shutdownVotes was 200 (allowing the collection shutdown to proceed). If the totalSupply then increases due to other actions, making cancellation possible, the process resets all parameters and shutdownVotes to zero.
When users attempt to reclaim their votes after cancellation, they may encounter underflow errors because their previous votes are no longer tracked.
If the collection becomes eligible for shutdown again, shutdownVotes will incorrectly reflect zero previous votes, resulting in some supply being locked without proper voting power accounting.

Impact

Prevents users from reclaiming their votes due to underflow revert and results in inconsistencies in the shutdown state.

Code Snippet

CollectionShutdown::cancel()

File: CollectionShutdown.sol
390:     function cancel(address _collection) public whenNotPaused {
391:         // Ensure that the vote count has reached quorum
392:         CollectionShutdownParams memory params = _collectionParams[_collection];
393:         if (!params.canExecute) revert ShutdownNotReachedQuorum();
394: 
395:         // Check if the total supply has surpassed an amount of the initial required
396:         // total supply. This would indicate that a collection has grown since the
397:         // initial shutdown was triggered and could result in an unsuspected liquidation.
398:         if (params.collectionToken.totalSupply() <= MAX_SHUTDOWN_TOKENS * 10 ** locker.collectionToken(_collection).denomination()) {
399:             revert InsufficientTotalSupplyToCancel();
400:         }
401: 
402:         // Remove our execution flag
403:         delete _collectionParams[_collection];
404:         emit CollectionShutdownCancelled(_collection);
405:     }

CollectionShutdown::reclaimVote()

File: CollectionShutdown.sol
356:     function reclaimVote(address _collection) public whenNotPaused {
---
368:         // We delete the votes that the user has attributed to the collection
369:@>       params.shutdownVotes -= uint96(userVotes);
---
377:     }

CollectionShutdown::_collectionParams mapping

File: CollectionShutdown.sol
60:     /// Maps a collection to it's respective shutdown parameters
61:     mapping (address _collection => CollectionShutdownParams _params) private _collectionParams;

ICollectionShutdown::CollectionShutdownParams struct

File: ICollectionShutdown.sol
43:     struct CollectionShutdownParams {
44:         uint96 shutdownVotes;
45:         address sweeperPool;
46:         uint88 quorumVotes;
47:         bool canExecute;
48:         ICollectionToken collectionToken;
49:         uint availableClaim;
50:         uint[] sweeperPoolTokenIds;
51:     }

Tool used

Manual Review

Recommendation

Modify the CollectionShutdown::cancel() function to preserve the shutdownVotes data. When restarting the collection shutdown process, ensure that shutdownVotes is not zero, which implies that this parameter is used to track each-time when a shutdown collection is started.

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:     }

This may necessitate additional variables or mechanisms to correctly track and handle cancellations and votes throughout the lifecycle of the shutdown process.

+ mapping(address _collection => uint96 unclaimedVote) public unclaimedVotes;
---
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()) {
        revert InsufficientTotalSupplyToCancel();
    }

+   unclaimedVotes[_collection] = params.shutdownVotes;
    // Remove our execution flag
    delete _collectionParams[_collection];
    emit CollectionShutdownCancelled(_collection);
}

+ mapping(address _collection => uint96 unclaimedVote) public unclaimedVotes;
---
function _vote(address _collection, CollectionShutdownParams memory params) internal returns (CollectionShutdownParams memory) {
    // Take tokens from the user and hold them in this escrow contract
    uint userVotes = params.collectionToken.balanceOf(msg.sender);
    if (userVotes == 0) revert UserHoldsNoTokens();

    // Pull our tokens in from the user
    params.collectionToken.transferFrom(msg.sender, address(this), userVotes);

    // Register the amount of votes sent as a whole, and store them against the user
    params.shutdownVotes += uint96(userVotes);
+   if (unclaimedVotes[_collection] != 0) { // Settle the unclaimed
+       params.shutdownVotes += unclaimedVotes[_collection];
+       unclaimedVotes[_collection] = 0;
+   }

    // Register the amount of votes for the collection against the user
    unchecked { shutdownVoters[_collection][msg.sender] += userVotes; }

    emit CollectionShutdownVote(_collection, msg.sender, userVotes);

    // If we can execute, then we need to fire another event
    if (!params.canExecute && params.shutdownVotes >= params.quorumVotes) {
        params.canExecute = true;
        emit CollectionShutdownQuorumReached(_collection);
    }

    return params;
}
---
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

    // Get the amount of votes that the user has cast for this collection
    uint userVotes = shutdownVoters[_collection][msg.sender];

    // If the user has not cast a vote, then we can revert early
    if (userVotes == 0) revert NoVotesPlacedYet();

    // We delete the votes that the user has attributed to the collection
-   params.shutdownVotes -= uint96(userVotes);
+   params.shutdownVotes != 0 ? params.shutdownVotes -= uint96(userVotes) : unclaimedVotes[_collection] -= uint96(userVotes); // Imply that if no `shutdownVotes` it is move to unclaimedVotes and not yet start again
    delete shutdownVoters[_collection][msg.sender];

    // We can now return their tokens
    params.collectionToken.transfer(msg.sender, userVotes);

    // Notify our stalkers that a vote has been reclaimed
    emit CollectionShutdownVoteReclaim(_collection, msg.sender, userVotes);
}

sherlock-audit / 2024-08-flayer-judging

merlinboii - Unable to reclaim votes after collection shutdown cancellation #727