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

TessKimy - Miscalculation of earned token amount may cause loss of funds #277

Open
sherlock-admin2 opened this issue Nov 13, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 13, 2024

TessKimy

High

Miscalculation of earned token amount may cause loss of funds

Summary

Miscalculation of earned token amount may cause loss of yields

Root Cause

In RewardAccrualBase, _earned() function is used for calculation of user's earned rewards. Earned rewards is calculated as follows:

    function _earned(address account) internal view virtual returns (uint256 earned) {
        RewardAccrualBaseStorageV0 storage $ = _getRewardAccrualBaseDataStorage();
        uint256 accountBalance = balanceOf(account);
        uint256 rewardDelta = $.rewardPerTokenStored - $.lastRewardPerTokenUsed[account];
        earned = accountBalance.mulDiv(rewardDelta, 1e24, Math.Rounding.Floor) + $.rewards[account]; // 1e24 for precision loss
    }

accountBalance is calculated using balanceOf function and as we know that reward per token is calculated using totalStaked() value

            } else {
                uint256 end = Math.min(block.timestamp, $.periodFinish);
                if ($.lastUpdateTime < end) {
                    timeElapsed = end - $.lastUpdateTime;
                } else {
                    timeElapsed = 0;
                }
            }
            uint256 rewardIncrease = $.rewardRate * timeElapsed;
&>          rewardPerToken = $.rewardPerTokenStored
                + rewardIncrease.mulDiv(1e24, totalStaked(), Math.Rounding.Floor); // 1e6 for precision loss

This calculation is wrong because totalStaked() represents total staked usualS token amount but balanceOf function is also including the unclaimed original allocation.

    function balanceOf(address account) public view override returns (uint256) {
        UsualSPStorageV0 storage $ = _usualSPStorageV0();
 &>     return
            $.liquidAllocation[account] + $.originalAllocation[account] - $.originalClaimed[account];
    }

This situation will cause loss of yield for the last reward claimer due to misaccounting.

Attack Path

  1. Total staked usualS token = 100, we have 5 users and 20 token staked by each of them and 1st user also have unclaimed original allocation which is 20 unit
  2. 100 unit of token is added as reward
  3. At the end of the reward period, first 4 user claimed their rewards
  4. 1st user is claimed 40 unit of token because his balance is 20 staked + 20 original allocation (unclaimed ) and the others claimed 20 unit of token
  5. 5th user cannot claim his rewards because there is no token left, first 4 user claimed all the tokens.

Impact

High - It will cause direct loss of yield which is high severity issue due to wrong calculation yields are directly impacted.

Mitigation

Also add original allocations to denominator while the calculation of reward per token

@sherlock-admin4 sherlock-admin4 changed the title Old Brunette Puma - Miscalculation of earned token amount may cause loss of funds TessKimy - Miscalculation of earned token amount may cause loss of funds Nov 19, 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