sherlock-audit / 2024-08-flayer-judging

2 stars 0 forks source link

almantare - The possibility of calling execute a second time results in a loss of funds #791

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

almantare

High

The possibility of calling execute a second time results in a loss of funds

Summary

canExecute in collectionShutdown can be set to true a second time, which means cancel can be called for a collection that has already had execute called on it. This leads to the deletion of the entry in the _collectionParams mapping, resulting in a DoS of the claim and voteAndClaim functions, and overall blocking of user funds, because after cancel it's impossible to return votes.

Vulnerability Detail

After execute has been called for a collection, the params.canExecute parameter is changed from true to false. The cancel function can be called by anyone, it only requires canExecute to be true, and totalSupply > 4 (Let's assume this function is executed in the given reasoning).

Since the protocol allows users to vote for liquidation even after the execute function is called, users can call the vote function, in which under certain conditions canExecute is changed to true again.

if (!params.canExecute && params.shutdownVotes >= params.quorumVotes) {
            params.canExecute = true;
            emit CollectionShutdownQuorumReached(_collection);
}

This means a scenario is possible (which can happen accidentally or be orchestrated by an attacker, where canExecute will be true again).

Once canExecute becomes true again, cancel can be called, which in turn will clear the _collectionParams[_collection] structure:

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/utils/CollectionShutdown.sol#L390-L405 This will lead to a DoS of the claim and voteAndClaim functions as the functions will revert on the following line of code:

if (params.sweeperPool == address(0)) revert ShutdownNotExecuted();

which in turn will lead to the blocking of native tokens that are intended as rewards for users. Thus, for each user, the native token will be blocked in the amount of amount (uint amount = params.availableClaim * userVotes / (params.quorumVotes * ONE_HUNDRED_PERCENT / SHUTDOWN_QUORUM_PERCENT)).

Moreover, there is currently an error in the protocol that prevents users from returning tokens they voted with after calling cancel. It is described in detail in the issue titled: There is no method of returning votes in case of cancellation, resulting in loss of funds

Thus, the root cause of the problem is the ability to call cancel after execute.

Impact

In this scenario, user funds and native tokens are blocked. It's not difficult for an attacker to achieve this scenario under conditions of low volatility and totalSupply of collections close to liquidation.

Severity: High

Code Snippet

Tool used

Manual Review

Recommendation

Add params.isExecuted, and check that param in cancel.