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

Ace-30 - [Carousel] Users lose money in mintRollovers because wrong calculation of assetsToMint #445

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

Ace-30

high

[Carousel] Users lose money in mintRollovers because wrong calculation of assetsToMint

Summary

When a user deposits to an epoch, they receive the same amount of share tokens, meaning that each share is worth one deposited token.
However, after winning the epoch, the value of the shares increases.

BUT

In the mintRollovers mechanism, the value of the shares is not taken into account and the user exchanges 100 shares of their old epoch for 100 shares of the new epoch, which have lesser value.
_mintShares(queue[index].receiver, _epochId, assetsToMint)

mintRollovers must consider entitledShares for minting new shares (like withdraw function)

Vulnerability Detail

Consider these steps:
for simplicity suppose that relayerFee=0

  1. user deposits 100 USDC to epoch A
  2. user gets 100 tokens of epochA
  3. user wins the epoch and can get 1000 USDC if withdraw his shares
  4. user enlists in rollover with 100 epochA tokens
  5. user calls mintRollovers for epoch B
    a. 100 epochA tokens are burnt:
    _burn( queue[index].receiver, queue[index].epochId, queue[index].assets );
    b. 100 epochB tokens are minted:
    uint256 assetsToMint = queue[index].assets - relayerFee;
    _mintShares(queue[index].receiver, _epochId, assetsToMint);
  6. 100 epochB tokens worth 100 USDC and the user has lost 900 USDC.
    (user could withdraw 1000USDC from epochA and deposit them to epochB and get 1000 shares of epochB)

Impact

user loses money because of wrong calculations in mintRollovers function

Code Snippet

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/Carousel/Carousel.sol#L436-L437

Tool used

Manual Review

Recommendation

Use entitled shares to calculate mint amount (like withdraw function):

uint256 assetsToMint = entitledShares - relayerFee;

instead of this line:

uint256 assetsToMint = queue[index].assets - relayerFee;

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