PeterSR - Start new voting period could be called during active period

[sherlock-admin3]

PeterSR

Medium

Start new voting period could be called during active period

Summary

In the current implementation, there is a missing check for ensuring that a new voting period cannot be started while an existing voting period is still ongoing. This can lead to inconsistencies in the voting process.

Vulnerability Detail

The vulnerability arises from the absence of a check in the Voter::startNewVotingPeriod function to verify whether the current voting period has ended before starting a new one. Without this check, a new voting period can be started even if the previous one has not concluded, leading to potential inconsistencies in the voting process.

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

Proof of Concept

Add this test to Voter.t.sol:

function testStartVotingPeriodWhenOldIsNotFinished() public {
    vm.prank(DEV);
    _voter.startNewVotingPeriod();

    assertEq(1, _voter.getCurrentVotingPeriod());

    (uint256 startTime, uint256 endTime) = _voter.getPeriodStartEndtime(1);
    assertGt(endTime, startTime);

    vm.prank(DEV);
    vm.expectRevert();
    _voter.startNewVotingPeriod();
}

Impact

• Inability to Use Votes: Users may be unable to cast their votes correctly if the voting periods overlap or are not properly managed.

• Loss of Rewards: Bribers may not allocate rewards for votes that are cast during overlapping or improperly managed voting periods. This can lead to users missing out on rewards they would have otherwise earned.

Code Snippet

function startNewVotingPeriod() public onlyOwner {
        _currentVotingPeriodId++;

        VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
        period.startTime = block.timestamp;
        period.endTime = block.timestamp + _periodDuration;

        emit VotingPeriodStarted();
    }

Tool used

Manual Review

Recommendation

Add this line of code in the Voter::startNewVotingPeriod function:

function startNewVotingPeriod() public onlyOwner {
+        require(
+            _currentVotingPeriodId == 0 || _votingEnded(),
+            "Voting is still ongoing"
+        );
        _currentVotingPeriodId++;

        VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
        period.startTime = block.timestamp;
        period.endTime = block.timestamp + _periodDuration;

        emit VotingPeriodStarted();
}

[WangSecurity]

Invalid based on the comment #166 (comment).