sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

John_Femi - Vote can be started in the middle of a previous vote #692

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

John_Femi

Medium

Vote can be started in the middle of a previous vote

Summary

Votes epoch can be changed in the middle of a vote

Vulnerability Detail

As seen in the code https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L107-L114 , there is no check if there is an active voting period before setting new voting period, this could lead to new voters intending to vote in a current epoch transferred to another epoch

Impact

Accounting issues on votes weights and counts and unfair advantage to user who voted in the last epoch

this can also lead to sandwiching of the startNewVotingPeriod to gain unfair advantage like this flow Stake -> Vote -> startNewVotingPeriod -> Vote -> Withdraw

Code Snippet

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

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

0xSmartContract commented 4 months ago

The startNewVotingPeriod function being called during the current voting period is not a valid vulnerability, as it is only done by the onlyOwner and is considered an "Admin Input/call validation". This is more of a situation where the admin has a duty to control.

https://docs.sherlock.xyz/audits/judging/judging#vii.-list-of-issue-categories-that-are-not-considered-valid

Admin Input/call validation:

Admin could have an incorrect call order. Example: If an Admin >forgets to setWithdrawAddress() before calling withdrawAll() This is >not a valid issue. An admin action can break certain assumptions about the functioning >of the code. Example: Pausing a collateral causes some users to be >unfairly liquidated or any other action causing loss of funds. This is >not considered a valid issue.