Skip to content
This repository has been archived by the owner on Jan 12, 2025. It is now read-only.

Silvermist - Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards #643

Closed
sherlock-admin3 opened this issue Jul 15, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Jul 15, 2024

Silvermist

High

Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards

Summary

Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards.

Vulnerability Detail

A malicious user can exploit the claim function to repeatedly update the _lastUpdateTimestamp.

This manipulation affects the _calculateRewards function, which uses the _lastUpdateTimestamp to determine the rewards.

When another user tries to vote and deposit, the rewards calculation will be based on a much shorter duration than expected, significantly reducing the rewards they receive.

Example scenario:

  • Malicious user participates in period 1. Let's say that this period lasts 10 days.
  • Period 2 starts, user1 claims his rewards with claim and by doing that he sets the _lastUpdateTimestamp to the beginning of period2. Let's say that period 2 lasts from 10th to the 20th day so again 10 days.
  • On the day15 i.e day 5 of period2, a user deposits.

Because of the way the rewards are calculated in _calculateRewards he should receive rewards for 5 days:

        uint256 timestamp = block.timestamp > endTime ? endTime : block.timestamp;

        return timestamp > lastUpdateTimestamp ? (timestamp - lastUpdateTimestamp) * emissionsPerSecond : 0;

where block.timestamp is not > than endTime since period 2 is still going. so timestamp = block.timestamp

In the 2nd condition:
timestamp > lastUpdatedTimestamp is true, then the rewards he should get is block.timestamp - lastUpdateTimestamp(currently beginning of the period2)) * emissions. So he should receive rewards for 5 days.

However, the malicious user can continuously claim rewards even if he doesn't have anything to claim, because that will update _lastUpdateTimestamp constantly.

Therefore, when the victim user claims, his reward will be calculated as block.timestamp - lastUpdateTimestamp and this will result in only a couple of seconds period for which he'll receive rewards.

Impact

Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/rewarders/BribeRewarder.sol#L300-L313

Tool used

Manual Review

Recomendation

Implement a mapping that stores the _lastUpdateTimestamp for each user individually.

Duplicate of #52

@github-actions github-actions bot added duplicate High A High severity issue. labels Jul 21, 2024
@sherlock-admin2 sherlock-admin2 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Quick Mahogany Viper - Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards Silvermist - Malicious user can manipulate reward calculations, preventing other users from receiving the correct amount of rewards Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
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 High severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants