sherlock-audit / 2024-08-flayer-judging

0 stars 0 forks source link

Rough Corduroy Eagle - Vote Manipulation in `voteAndClaim` Function #784

Open sherlock-admin4 opened 4 days ago

sherlock-admin4 commented 4 days ago

Rough Corduroy Eagle

High

Vote Manipulation in voteAndClaim Function

Summary

Flayer Collection Shutdown: Vote Manipulation Bug

1. Title: Vote Manipulation in voteAndClaim Function

2. Why this could be triggered?

The voteAndClaim function is designed for efficiency, allowing users to participate in a collection shutdown vote and claim their share of the liquidation proceeds in a single transaction. However, a logical error in the function allows for vote manipulation. This error occurs because the user's votes are burned without being added to the total shutdown votes count, allowing users to skew the claim calculation in their favor.

3. PoC Flow to Trigger:

  1. Trigger Collection Shutdown: A user initiates the shutdown process for a collection, reaching the required quorum.
  2. Liquidation Complete: All NFTs in the Sudoswap pool are sold, making funds available for claim.
  3. Call voteAndClaim Repeatedly: A user with dust tokens of the collection repeatedly calls voteAndClaim. Each call:
    • Burns their existing dust tokens as a vote (without updating total shutdown votes).
    • Claims a share based on a proportion that keeps growing with each call, due to the underreported shutdownVotes in the calculation.

4. Impact in Full Details:

5. Function Code:

    function voteAndClaim(address _collection) public whenNotPaused {
        // Ensure that we have moved token IDs to the pool
        CollectionShutdownParams memory params = _collectionParams[_collection]; 
        if (params.sweeperPool == address(0)) revert ShutdownNotExecuted();

        // Ensure that all NFTs have sold from our Sudoswap pool
        if (!collectionLiquidationComplete(_collection)) revert NotAllTokensSold();

        // Take tokens from the user and hold them in this escrow contract
        uint userVotes = params.collectionToken.balanceOf(msg.sender);
        if (userVotes == 0) revert UserHoldsNoTokens();
        params.collectionToken.burnFrom(msg.sender, userVotes);

        // We can now delete our sweeper pool tokenIds
        if (params.sweeperPoolTokenIds.length != 0) {
            delete _collectionParams[_collection].sweeperPoolTokenIds;
        }

        // Get the number of votes from the claimant and the total supply and determine from that the percentage
        // of the available funds that they are able to claim.
        uint amount = params.availableClaim * userVotes / (params.quorumVotes * ONE_HUNDRED_PERCENT / SHUTDOWN_QUORUM_PERCENT);
        (bool sent,) = payable(msg.sender).call{value: amount}('');
        if (!sent) revert FailedToClaim();

        emit CollectionShutdownClaim(_collection, msg.sender, userVotes, amount);
    }

Recommendation:

To fix this bug, the function needs to correctly account for the new vote by adding the userVotes to the params.shutdownVotes before calculating the claim amount. This will ensure fair distribution of funds among participants during the collection shutdown process.

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

No response