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

9 stars 8 forks source link

nirohgo - Reward emissions can be blocked with a DOS attack due to insufficient precision is emissions calculation #90

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

nirohgo

High

Reward emissions can be blocked with a DOS attack due to insufficient precision is emissions calculation

Summary

A malicious user can cause emission-based rewards to be lost by making frequent calls to claimAccountRewards triggering emission calculations for small enough emissions that are rounded down to zero.

Vulnerability Detail

The vaultRewarderLib calculates reward emissions in the following way:

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

The emmissions are calculated whenever a user calls claimAccountRewards (without any limit on the frequency of calls). If a malicious user calls claimAccountRewards frequently enough (see example below) they can trigger emission accumulations at points where the emission value is rounded down to zero.

attack example

A. a reward emission is set for a vault of 10,000 USDC over a year.
B. The vault has 100000 shares.
C. A malicious user calls claimAccountRewards every 100 blocks (on Arbitrum: roughly every 200 seconds).
Based on the code formula the additional emission calculation is:
INTERNAL_TOKEN_PRECISION = 10^8
USDC emissionRatePerYear = 10000 10^6
timeSinceLastAccumulation = 200
Constants.YEAR = 31536000
totalVaultSharesBefore = 100000
10^8
Emmission = 200 10^8 10000 10^6 / (31536000 100000 * 10^8) = 0.63
D. Since the result is a fraction it gets rounded down to zero, however the emission is counted as successful and the next emission calculation will start from the current time.
E. As the attacker continues to call the function at the needed frequency, the reward never gets accumulated.
F. The specific interval of calls required for the attack depends on the reward amount, decimals and the number of shares in the vault.

Impact

Loss of rewards for vault users and incentive providers.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add a precision factor to the state.accumulatedRewardPerVaultShare variable (i.e. it is always kept in 10^36 precision regardless of the reward token decimals). Adjust back to the token decimals in _claimRewardToken where the value is sent to the claiming user.

Duplicate of #61

sherlock-admin3 commented 2 months ago

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

0xmystery commented:

Low/QA at most due to dust amount loss

nirohgo commented 2 months ago

Escalate

Please note that the attack eliminates the entire reward ($10,000 in the example above) and not just dust. While every individual attacker call to claimAccountRewards eliminates a very small amount (rounded down to zero) the attacker calls the function every 100 blocks until the end of the reward duration, which in total eliminates the entire reward.

sherlock-admin3 commented 2 months ago

Escalate

Please note that the attack eliminates the entire reward ($10,000 in the example above) and not just dust. While every individual attacker call to claimAccountRewards eliminates a very small amount (rounded down to zero) the attacker calls the function every 100 blocks until the end of the reward duration, which in total eliminates the entire reward.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

mystery0x commented 1 month ago

Escalate

Please note that the attack eliminates the entire reward ($10,000 in the example above) and not just dust. While every individual attacker call to claimAccountRewards eliminates a very small amount (rounded down to zero) the attacker calls the function every 100 blocks until the end of the reward duration, which in total eliminates the entire reward.

Will be duped to #61 if the latter is ever confirmed by the sponsor subject to the Sherlock judge decision because:

This report did not stress the feasibility "On the L2 environment (e.g., Arbitrum), due to low transaction fees" as #61 did.

WangSecurity commented 1 month ago

Based on my comment under 62 I believe this issue is possible even without a malicious intent and I don't think not mentioning Arbitrum and low fees makes it insufficient to be duplicated.

Planning to accept the escalation and duplicate with #61.

WangSecurity commented 1 month ago

Result: High Duplicate of #61

sherlock-admin4 commented 1 month ago

Escalations have been resolved successfully!

Escalation status: