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

dipp - Incorrect inflation index due to incorrect calculation of totalStaked #121

Closed
sherlock-admin opened this issue Nov 4, 2022 · 0 comments

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Nov 4, 2022

dipp

high

Incorrect inflation index due to incorrect calculation of totalStaked

Summary

The gInflationIndex in Comptroller.sol variable could be incorrect due to totalFrozen being subtracted from the totalStaked twice in the withdrawRewards function.

Vulnerability Detail

In the withdrawRewards function of Comptroller.sol, the _getUserManagerState function is called which will return totalStaked and totalFrozen values. The totalStaked returned by _getUserManagerState is calculated by subtracting the userManager.totalFrozen from the userManager.totalStaked and the totalFrozen returned is the userManager.totalFrozen. The withdrawRewards function will then calculate a new totalStaked value by subtracting userManagerState.totalFrozen from the userManagerState.totalStaked and use this new value in the _getInflationIndexNew function to get a new value for gInflationIndex.

Impact

The gInflationIndex variable could be updated incorrectly which could lead to incorrect reward calculations.

Code Snippet

Comptroller.sol:_getUserManagerState#L306-L316:

    function _getUserManagerState(IUserManager userManager) internal view returns (UserManagerState memory) {
        UserManagerState memory userManagerState;

        userManagerState.totalFrozen = userManager.totalFrozen();
        userManagerState.totalStaked = userManager.totalStaked() - userManagerState.totalFrozen;
        if (userManagerState.totalStaked < 1e18) {
            userManagerState.totalStaked = 1e18;
        }

        return userManagerState;
    }

The getUserManagerState function gets the userManager's totalFrozen and calculates the totalStaked as the userManager's totalStaked - its totalFrozen.

Comptroller.sol:withdrawRewards#L260-L264:

        uint256 totalStaked_ = userManagerState.totalStaked - userManagerState.totalFrozen;
        gInflationIndex = _getInflationIndexNew(totalStaked_, block.number - gLastUpdatedBlock);
        gLastUpdatedBlock = block.number;
        users[account][token].updatedBlock = block.number;
        users[account][token].inflationIndex = gInflationIndex;

In the withdrawRewards function, the totalStaked used to claculate the new inflationIndex is calculated as the userManagerState's totalStaked - totalFrozen again.

Tool used

Manual Review

Recommendation

In the withdrawRewards function, do not subtract totalFrozen from the totalStaked again.

Duplicate of #26

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant