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

Rewards are not rolled over #93

Open
code423n4 opened this issue Sep 16, 2022 · 1 comment
Open

Rewards are not rolled over #93

code423n4 opened this issue Sep 16, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working selected for report This submission will be included/highlighted in the audit report sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/rewards/StakingRewards.sol#L183

Vulnerability details

Impact

If there is no deposit for sometime in start then reward for those period is never used

Proof of Concept

  1. Admin has added reward which made reward rate as 10 reward per second using notifyRewardAmount function
rewardRate = reward.div(rewardsDuration);
  1. For initial 10 seconds there were no deposits which means total supply was 0
  2. So no reward were distributed for initial 10 seconds and reward for this duration which is 10*10=100 will remain in contract
  3. Since on notifying contract of new rewards, these stuck rewards are not considered so these 100 rewards will remain in contract with no usage

Recommended Mitigation Steps

On very first deposit better to have (block.timestamp-startTime) * rewardRate amount of reward being marked unused which can be used in next notifyrewardamount

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 16, 2022
code423n4 added a commit that referenced this issue Sep 16, 2022
@MiguelBits MiguelBits added the invalid This doesn't seem right label Sep 30, 2022
@liveactionllama liveactionllama added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue and removed invalid This doesn't seem right labels Oct 3, 2022
@HickupHH3
Copy link
Collaborator

I see this as protocol leaked value since the rewards would be "lost" and isn't attributed to anyone.

Currently, the sweeper function allows the reward token to be withdrawn, thus providing a form of recovery. However, #49 and its dups points out that this is a vuln, and if fixed, will remove this recovery.

@HickupHH3 HickupHH3 added the selected for report This submission will be included/highlighted in the audit report label Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working selected for report This submission will be included/highlighted in the audit report sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

4 participants