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

charlesjhongc - Incorrect mint amount is used in mintRollovers() #223

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Closed
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

charlesjhongc

high

Incorrect mint amount is used in mintRollovers()

Summary

mintRollovers() use incorrect mint amount which will cause part of funds being locked in a vault forever.

Vulnerability Detail

The rollover mint should be equal to withdraw() plus deposit(). The assets in the rollover queue represent how much share of an epoch that an user wants to rollover. The rollover gets minted only if user won the epoch he is rolling over. That means a user can withdraw more underlying token from the epoch then he deposited. So the rollover should be minted using the amount of previewWithdraw(queue[index].epochId, queue[index].assets) rather than queue[index].assets. Using queue[index].assets indicate that the extra bonus is not counted and will be left in vault forever (since each epoch has it's own finalTVL/claimTVL which is isolated from other epochs).

Impact

  1. User will not be able to rollover with correct amount (entitledShares).
  2. Part of extra underlying token of an epoch will get locked in a vault forever.

Code Snippet

function mintRollovers(uint256 _epochId, uint256 _operations)
{
    // ...
    // ...
    while (...) {
        // only roll over if last epoch is resolved
        if (epochResolved[queue[index].epochId]) {
            uint256 entitledShares = previewWithdraw(
                queue[index].epochId,
                queue[index].assets
            );
            // mint only if user won epoch he is rolling over
            if (entitledShares > queue[index].assets) {
                _burn(...);
                _burnEmissions(...);
                emissionsToken.safeTransfer(...);
                emit Withdraw(...);

                // @audit should be entitledShares - relayerFee
                uint256 assetsToMint = queue[index].assets - relayerFee;
                _mintShares(queue[index].receiver, _epochId, assetsToMint);

                emit Deposit(...);
                rolloverQueue[index].assets = assetsToMint;
                rolloverQueue[index].epochId = _epochId;
                // only pay relayer for successful mints
                executions++;
            }
        }
        index++;
    }
    // ...
    // ...
}

Tool used

Manual Review

Recommendation

Calculate assetsToMint in mintRollovers() using entitledShares instead.

Record rollover queue index only when an item is pushed into queue.

uint256 assetsToMint = entitledShares - 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