Users can open positions with zero lock duration and still earn rewards

Summary

createPosition() in MLUMStaking.sol allows users to open staking positions with zero lock duration. They cannot vote using these positions but they can still earn and claim rewards (only 1x multiplier) even though they have not really locked anything.

This defeats the purpose of staking rewards, a way of incentivizing users to "lock" their MLUM, because any user can game this by opening positions with large amounts of MLUM, get portions of reward and they can withdraw instantly whenever they want.

Vulnerability Detail

When we check the createPosition() flow, there is nothing stopping users from opening staking positions with zero lock duration. They will get an NFT minted, will accrue rewards (with only 1x multiplier) and can claim their portion of rewards distributed. Everything will work normally for them, and nothing prevents them from keeping the position locked with the same duration, and keep harvesting rewards from it, and withdraw when they feel like.

The only thing is they will not be able to participate in voting (due to checks in Voter.sol contract) but they can surely steal a portion of the rewards on MLUMStaking itself.

See createPosition() : a new position with lockDuration = 0 simply goes through normally. Its amountWithMultiplier gets calculated as = amount (ie. 1x).

This is the harvest function :


    function _harvestPosition(uint256 tokenId, address to) internal {
        StakingPosition storage position = _stakingPositions[tokenId];

        // compute position's pending rewards
        uint256 pending = position.amountWithMultiplier * _accRewardsPerShare / PRECISION_FACTOR - position.rewardDebt;

        if (pending > 0) {
            _safeRewardTransfer(to, pending);
        }
        emit HarvestPosition(tokenId, to, pending);
    }

The rewards earned depend on the position's amountWithMultiplier and have no checks or association with the lockDuration of the position.

This is unfair to users who are actually locking their assets for a tangible duration. If many users do this at the same time with large amounts, collectively most of the rewards distributed might go to zero lock durations, which is again worse for the other lockers because they deserve those rewards.

Impact

Users who actually "lock" assets lose out some share of the rewards to unfair lockers who do not really "lock" the assets. This should never happen.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L354

Tool used

Manual Review

Recommendation

Have a minimum lock duration set by the admin in MLUMStaking and only allow positions to be created when they pledge above the minimum lock duration. This could be set to 1 week for example.

Duplicate of #74