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

Reentrants - Check for lockup past the voting epoch is incorrectly implemented. #240

Closed
sherlock-admin2 opened this issue Jul 15, 2024 · 0 comments
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

Medium

Check for lockup past the voting epoch is incorrectly implemented.

Summary

The MagicSea contract has a voting functionality where users vote on reward distribution with their staked tokens. The current implementation checks the lockDuration to ensure tokens are locked beyond the voting period, preventing double-voting. However, it incorrectly uses the total lock duration instead of the remaining lock time, allowing users to double-vote if their total lock duration exceeds the voting period. This flaw enables users to bypass the intended restrictions and double-vote on proposals.

Vulnerability Detail

The MagicSea contract implements a voting functionality, which allows users to vote on reward distribution with their staked tokens. To ensure that no double votes are possible, the documentation describes that stakes will be checked if they are locked up until after the voting period has passed. This is done to ensure that users are not able to double-vote.

. The overall lock needs to be longer then 90 days and the remaining lock period needs to be longer then the epoch time.

This is implemented in the following way in code.

if (_mlumStaking.getStakingPosition(tokenId).lockDuration < _periodDuration) {
    revert IVoter__InsufficientLockTime();
}

This code is incorrectly implemented as it checks the lockDuration parameter to be bigger than the _periodDuration. The key aspect that this is missing is that the lock duration parameter does not show the remaining lock time but the total lock time. It is set on the creation of the position and never changed unless the user extends his position. As a result, any position with a lockDuration that is more significant than _periodDuration will forever pass this requirement.

Impact

The issue results in users being able to double-vote on proposals.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/Voter.sol#L175-L177

Tool used

Manual Review

Recommendation

We recommend adapting the check to ensure that the position is locked until the voting period has passed: check it in the following way.

if (_mlumStaking.getStakingPosition(tokenId).startTime + _mlumStaking.getStakingPosition(tokenId).lockDuration - block.timestamp < _periodDuration - (block.timestamp - startTime[currentVotingPeriod]) {
    revert IVoter__InsufficientLockTime();
}

Duplicate of #166

@github-actions github-actions bot added duplicate High A High 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
@sherlock-admin4 sherlock-admin4 changed the title Lone Opaque Mustang - Check for lockup past the voting epoch is incorrectly implemented. Reentrants - Check for lockup past the voting epoch is incorrectly implemented. Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
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

2 participants