sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

zhuying - `claimAccountRewards` could revert if many users want to claim at the same time #102

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

zhuying

Medium

claimAccountRewards could revert if many users want to claim at the same time

Summary

claimAccountRewards could revert if many users want to claim at the same time.

Vulnerability Detail

If a user want to claim reward, he call claimAccountRewards. _claimAccountRewards will be called next. In `_claimAccountRewards:

if (0 < state[i].emissionRatePerYear) {
                // Accumulate any rewards with an emission rate here
                _accumulateSecondaryRewardViaEmissionRate(i, state[i], totalVaultSharesBefore);
            }

If emissionRatePerYear > 0, _accumulateSecondaryRewardViaEmissionRate will be called. additionalIncentiveAccumulatedPerVaultShare will be calculated.

            additionalIncentiveAccumulatedPerVaultShare =
                (timeSinceLastAccumulation
                    * uint256(Constants.INTERNAL_TOKEN_PRECISION)
                    * state.emissionRatePerYear)
                / (Constants.YEAR * totalVaultSharesBefore);

The average block time for Ethereum and Arbitrum is approximately 12 seconds. If many users want to claim at the same time, min of timeSinceLastAccumulation is 12s. Constants.YEAR = 86400 3600 = 31104000. It's very likely that the divisor (timeSinceLastAccumulation uint256(Constants.INTERNAL_TOKEN_PRECISION) state.emissionRatePerYear) is less than the dividend(Constants.YEAR totalVaultSharesBefore). So the function will revert leading to the claim failing.

Impact

claimAccountRewards could revert if many users want to claim at the same time.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/common/VaultRewarderLib.sol#L383-L387

Tool used

manual

Recommendation

Add check: if the divisor is less than the dividend , then additionalIncentiveAccumulatedPerVaultShare = 0.

-           dditionalIncentiveAccumulatedPerVaultShare = 
-           (timeSinceLastAccumulation 
-           * uint256(Constants.INTERNAL_TOKEN_PRECISION) 
-           * state.emissionRatePerYear) 
-           / (Constants.YEAR * totalVaultSharesBefore);  
+         uint256 divisor =
+         timeSinceLastAccumulation * uint256(Constants.INTERNAL_TOKEN_PRECISION) * state.emissionRatePerYear;
+         uint256 dividend = Constants.YEAR * totalVaultSharesBefore;
+         if (divisor < dividend) return state.accumulatedRewardPerVaultShare;
+         additionalIncentiveAccumulatedPerVaultShare = divisor / dividend;
sherlock-admin3 commented 2 months ago

2 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Low/QA at most on an intended design

Hash01011122 commented:

Low/Info, Chances of this to play in actual time is not possible.