Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

nobody2018 - If users won epoch he is rolling over, mintRollovers will cause these users  to lose funds #66

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

nobody2018

high

If users won epoch he is rolling over, mintRollovers will cause these users  to lose funds

Summary

The user deposits an asset in exchange for an equivalent amount of ERC1155 tokens. If the user wins epoch, his ERC1155 tokens can be exchanged for more than the asset he originally deposited. If the user loses epoch, his ERC1155 token is exchanged for less than the asset he originally deposited (maybe 0). mintRollovers mint only for who won epoch he is rolling over. However, mintRollovers just equally  exchanged ERC1155 tokens of the previous epoch for ERC1155 tokens of the new epoch. This is obviously unfair to users, and the assets won by users should be returned to users.

Vulnerability Detail

Let's assume a scenario to illustrate this issue:

  1. new round id is E1, bob deposited 1e18 asset into one collateralVault for E1, then he got 1e18 ERC1155 token(tokenid=E1).
  2. when epoch E1 ended without depeg event, premiumVault's asset was transferred to collateralVault. Assuming claimTVL[E1]/finalTVL[E1] = 120%.
  3. bob won this epoch. In other words, he will get 1.2e18 assets when he withdraws using 1e18 ERC1155 token.

The above is a normal situation. Now let's assume another scenario:

  1. new round id is E1, bob deposited 1e18 asset into one collateralVault for E1, then he got 1e18 ERC1155 token(id=E1).
  2. bob called enlistInRollover(E1, 1e18, bob)
  3. when epoch E1 ended without depeg event, premiumVault's asset was transferred to collateralVault. Assuming claimTVL[E1]/finalTVL[E1] = 120%. id of new epoch is E2.
  4. relayer called mintRollovers(E1, 50)  to process rolloverQueue
  5. bob only got 1e18 - relayerFee erc1155 token for E2.

Bob should have got 0.2e18 assets and 1e18 - relayerFee erc1155 token for E2. Therefore, bob loses 0.2e18 asset.

Impact

This issue will cause all users who won epoch in rolloverQueue to lose funds.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L396-L448

Tool used

Manual Review

Recommendation

--- a/Earthquake/src/v2/Carousel/Carousel.sol
+++ b/Earthquake/src/v2/Carousel/Carousel.sol
@@ -424,7 +424,10 @@ contract Carousel is VaultV2 {
                             queue[index].assets
                         )
                     );
-
+                    SemiFungibleVault.asset.safeTransfer(
+                        queue[index].receiver, 
+                        entitledShares - queue[index].assets
+                    );
                     emit Withdraw(
                         msg.sender,
                         queue[index].receiver,

Duplicate of #163

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant