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

Reentrants - Bribe rewards accumulated from start of voting period to first vote are lost #183

Closed
sherlock-admin2 opened this issue Jul 15, 2024 · 1 comment
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-admin2
Copy link

sherlock-admin2 commented Jul 15, 2024

Reentrants

High

Bribe rewards accumulated from start of voting period to first vote are lost

Summary

Unassigned rewards whilst no one has voted for a pool will be lost.

Vulnerability Detail

Rewards are streamed per second in BribeRewarder for the duration of the voting period. While no one has voted for that pool, the rewards remain unassigned and are lost, as there isn't a function to sweep up the rewards, and neither can it be rolled over to future voting periods because bribe() / fundAndBribe() can only be called once.

POC

Insert this test into BribeRewarder.t.sol.

function testLostRewards() public {
  ERC20Mock(address(rewardToken)).mint(address(this), 10e18);
  ERC20Mock(address(rewardToken)).approve(address(rewarder), 10e18);

  rewarder.fundAndBribe(1, 1, 10e18);

  _voterMock.setCurrentPeriod(1);
  _voterMock.setStartAndEndTime(100, 200);

  // join midway through voting period
  vm.warp(150);
  vm.prank(address(_voterMock));
  rewarder.deposit(1, 1, 1e18);

  // warp to end of vote
  vm.warp(200);
  _voterMock.setCurrentPeriod(2);
  _voterMock.setLatestFinishedPeriod(1);

  // only half of rewards are claimable
  vm.prank(alice);
  assertEq(5e18, rewarder.getPendingReward(1));
  rewarder.claim(1);

  // rewards are lost
  assertGt(rewardToken.balanceOf(address(rewarder)), 0);
}

Impact

Permanent loss of rewards.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/rewarders/BribeRewarder.sol#L278-L282
https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/rewarders/BribeRewarder.sol#L300-L313

Tool used

Manual Review

Recommendation

Rewards should only start streaming from the first vote, and in the extreme case where no one votes for a pool, implement sweeping rewards.

Duplicate of #172

@github-actions github-actions bot added duplicate Medium A Medium 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
@0xSmartContract 0xSmartContract added High A High severity issue. and removed Medium A Medium severity issue. labels Jul 29, 2024
@sherlock-admin4 sherlock-admin4 changed the title Lone Opaque Mustang - Bribe rewards accumulated from start of voting period to first vote are lost Reentrants - Bribe rewards accumulated from start of voting period to first vote are lost Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
@WangSecurity
Copy link

Now a duplicate of #172, based on #164 (comment) and #164 (comment) comments.

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

4 participants