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

coffiasd - starting new voting period not check current period has already ended #219

Closed
sherlock-admin2 opened this issue Jul 15, 2024 · 1 comment
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented Jul 15, 2024

coffiasd

High

starting new voting period not check current period has already ended

Summary

starting new voting period not check current period has already ended , which can lead to calculation error result in reward token lost.

Vulnerability Detail

code link

  function startNewVotingPeriod() public onlyOwner {
      _currentVotingPeriodId++;

      VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
      period.startTime = block.timestamp;
      period.endTime = block.timestamp + _periodDuration;//@audit staring new Period not ensure current has end.

      emit VotingPeriodStarted();
  }

From above code we can see there is no check if current period has ended.

when user vote, user -> Voter.sol ->BribeReward.sol to calculate the amount of reward token.

calculation code

    function _calculateRewards(uint256 periodId) internal view returns (uint256) {
        (uint256 startTime, uint256 endTime) = IVoter(_caller).getPeriodStartEndtime(periodId);

        if (endTime == 0 || startTime > block.timestamp) {
            return 0;
        }

        uint256 duration = endTime - startTime;//@audit startNewRound is ensure current round is end which can lead to reward calculation error.
        uint256 emissionsPerSecond = _amountPerPeriod / duration;

        uint256 lastUpdateTimestamp = _lastUpdateTimestamp;
        uint256 timestamp = block.timestamp > endTime ? endTime : block.timestamp;
        return timestamp > lastUpdateTimestamp ? (timestamp - lastUpdateTimestamp) * emissionsPerSecond : 0;
    }

the _amountPerPeriod is a fixed value after admin fund BribeReward.sol contract. Thus the emissions speed is based on endTime - startTime aka period duration.

If the administrator intentionally or unintentionally starts a new voting period early, it will result in the remaining reward tokens being stuck in the contract because the release rate of reward tokens is calculated based on the end time.

Impact

reward tokens being stuck in the contract

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L107-L115

Tool used

Foundry
Manual Review

Recommendation

check if current period has ended before starting new period

@@ -108,6 +108,7 @@ contract Voter is Ownable2StepUpgradeable, IVoter {
         _currentVotingPeriodId++;
 
         VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
+        require(block.timestamp > period.endTime);
         period.startTime = block.timestamp;
         period.endTime = block.timestamp + _periodDuration;
@github-actions github-actions bot added duplicate High A High severity issue. labels Jul 21, 2024
@sherlock-admin3 sherlock-admin3 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 Glorious Garnet Stallion - starting new voting period not check current period has already ended coffiasd - starting new voting period not check current period has already ended 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

Invalid based on the comment here.

@WangSecurity WangSecurity removed High A High severity issue. Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Aug 13, 2024
@sherlock-admin2 sherlock-admin2 added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue labels Aug 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

4 participants