sherlock-audit / 2024-08-flayer-judging

2 stars 0 forks source link

almantare - Incorrect uint88 and uint96 types leads to CollectionShutdownParams.quorumVotes and CollectionShutdownParams.shutdownVotes rounding Down #789

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

almantare

High

Incorrect uint88 and uint96 types leads to CollectionShutdownParams.quorumVotes and CollectionShutdownParams.shutdownVotes rounding Down

Summary

The CollectionShutdown contract uses the CollectionShutdownParams structure to store information about the collections that are voted on. The most interesting thing for us in it is the uint88 quorumVotes parameter.

From the code fragments below we can see that the dimension of quorumVotes is equal to the dimension of CollectionToken. The maximum dimensionality of CollectionToken is 27. 10 ** 27 does not fit into the uint88 type.

That's why the value of quorumVotes will be rounded down, which will break the protocol functionality.

A similar error occurs when calculating params.shutdownVotes. Only it does not fit into the uint96 type, but this error is very unlikely, unlike the first one. It is possible only in an extreme case, which will be described below.

Vulnerability Detail

QuorumVotes is the threshold value of tokens that users must vote to liquidate a collection in CollectionShutdown. QuorumVotes is calculated from the totalSupply of tokens at the current moment. The tokens are a CollectionToken contract that uses denomination as a floating user-specified parameter that increases the base accuracy of ERC20 - 18. denomination ≥ 0 and ≤ 9. Thus, the maximum dimensionality of collectionToken is 27.

As mentioned above, uint88 does not accommodate only CollectionToken with decimals 27 (denomination = 9). Let's show on a concrete example.

Let totalSupply of a collection = MAX_SHUTDOWN_TOKENS * 10 ^ denomination = 4 * 10 ^ 27

Then quorumVotes = 4 * 10 ^ 27 * 50 / 100 = 2 * 10 ^ 27.

The maximum value of uint88 is 2 ^ 88 - 1

2 * 10 ^ 27 > 2 ^ 88 - 1

Thus, collectionToken c denomination = 9 will overflow quorumVotes, indicating an incorrect vote threshold.

This error occurs in the code in two places. In the start function and in the execute function.

In the start function it is assumed that totalSupply ≤ 4, while in the execute function totalSupply can be anything.

Thus, rounding down is possible in both of these functions.

We should also add that the same error occurs when calculating params.shutdownVotes. Only here uint96 is used. This type stops accommodating tokens with denomination = 9 only when tokens ≥ ~ 80. Since this is used when counting user votes and the developers allow that the totalSupply of a collection can grow and become > 4 during the voting period - it is also necessary to consider the edge case that exactly during the voting period an illiquid collection with token totalSupply ≤ 4 and denomination = 9 will become very liquid (token ts≥ 80). And at least 80 tokens will be burned in the liquidation vote. Then uint96 will overflow. This is too unlikely an edge case, so we didn't put it into a separate error, but we should take it into account.

Impact

This type selection error violates the shutdownVotes logic. The following scenarios are possible. Assume everywhere that collectionToken denomination = 9.

Severity: High

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/utils/CollectionShutdown.sol#L135-L157

Tool used

Manual Review

Recommendation

Use: uint104 (uint96 unsafe too, because totalSuppply ≥ 160 in execute will overflow)