sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

quaternion - [Medium] Beginning the voting period can be delayed by miners in their interest (use of `block.timestamp` in `Voter::startNewVotingPeriod()` function) #659

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

quaternion

Medium

[Medium] Beginning the voting period can be delayed by miners in their interest (use of block.timestamp in Voter::startNewVotingPeriod() function)

Summary

The Voter::startNewVotingPeriod() function though can only be called by owner, but the function can be delayed arbitrarily by the miners.

Vulnerability Detail

The Voter::startNewVotingPeriod() function updates the starting period with block.timestamp to initiate new starting period. This function if postponed by miner, can delay the voting period. Putting a deadline on transaction allows the voting period to be limited by deadline.

Impact

This can break functionality as voting can start at arbitrary time as per miner and on-time execution may require owner to provide high amount of funds. But the user funds are not at risk. So medium severity can be assigned.

Code Snippet

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

Tool used

Manual Review

Recommendation

The owner shall specify the deadline by which the voting period has to start and the function shall revert in case block.timestamp is greater than that.

-   function startNewVotingPeriod() public onlyOwner {
+   function startNewVotingPeriod(uint256 deadline) public onlyOwner {
        _currentVotingPeriodId++;
+       require(deadline <= block.timestamp, "transaction delayed beyond deadline");
        VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
        period.startTime = block.timestamp;
        period.endTime = block.timestamp + _periodDuration;

        emit VotingPeriodStarted();
    }
0xSmartContract commented 3 months ago

This issue falls under the admin category

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

Admin Input/call validation:

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.