Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sherlock-admin4 opened this issue Sep 15, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 15, 2024

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.

  • QuorumVotes will overflow the uint88 type and round down. Thus the threshold of votes needed to eliminate the collection will be much smaller than needed, and any vote will overflow it.
  • Since the tokens that users vote to eliminate this collection will also have denomination = 9, any vote will overflow quorumVotes with type uint88. (10 ^ 27 > 2 ^ 88 - 1)
  • In execute, a collection no longer has a limit on totalSupply, so even collections with a large totalSupply (if it suddenly changes from ≤ 4) may be eliminated because of this bug

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)

@sherlock-admin2 sherlock-admin2 changed the title Cuddly Ocean Gibbon - Incorrect uint88 and uint96 types leads to CollectionShutdownParams.quorumVotes and CollectionShutdownParams.shutdownVotes rounding Down almantare - Incorrect uint88 and uint96 types leads to CollectionShutdownParams.quorumVotes and CollectionShutdownParams.shutdownVotes rounding Down Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant