dany.armstrong90 - An attacker can acquire much share of rewards by sandwiching transaction which transfers rewards to MlumStaking.

[sherlock-admin4]

dany.armstrong90

Medium

An attacker can acquire much share of rewards by sandwiching transaction which transfers rewards to MlumStaking.

Summary

There is no lowest limit of lockDuration in createPosition(). So an attacker can frontrun transfering rewards to MlumStaking with createPosition() and then he can instantly withdraw from that position.

Vulnerability Detail

MlumStaking.sol#createPosition() function which deposits funds and creates position is as follows.

    function createPosition(uint256 amount, uint256 lockDuration) external override nonReentrant {
        // no new lock can be set if the pool has been unlocked
        if (isUnlocked()) {
            require(lockDuration == 0, "locks disabled");
        }

        _updatePool();

        // handle tokens with transfer tax
        amount = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amount);
        require(amount != 0, "zero amount"); // createPosition: amount cannot be null

        // mint NFT position token
        uint256 currentTokenId = _mintNextTokenId(msg.sender);

        // calculate bonuses
        uint256 lockMultiplier = getMultiplierByLockDuration(lockDuration);
371     uint256 amountWithMultiplier = amount * (lockMultiplier + 1e4) / 1e4;

        // create position
        _stakingPositions[currentTokenId] = StakingPosition({
            initialLockDuration: lockDuration,
            amount: amount,
            rewardDebt: amountWithMultiplier * (_accRewardsPerShare) / (PRECISION_FACTOR),
            lockDuration: lockDuration,
            startLockTime: _currentBlockTimestamp(),
            lockMultiplier: lockMultiplier,
            amountWithMultiplier: amountWithMultiplier,
            totalMultiplier: lockMultiplier
        });

        // update total lp supply
        _stakedSupply = _stakedSupply + amount;
        _stakedSupplyWithMultiplier = _stakedSupplyWithMultiplier + amountWithMultiplier;

        emit CreatePosition(currentTokenId, amount, lockDuration);
    }

As we can see above, there is no lowest limit of lockDuration.
And MlumStaking.sol#_updatePool() function which distributes rewards is as follows.

    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);
    }

An attacker can see a transaction which transfers much rewards to MlumStaking in mempool. Then, he sandwiches that transaction with createPosition()(much funds) and withdrawFromPosition().
Here, even if lockDuration == 0, amountWithMultiplier = amount on L371.
So he can acquire much share of rewards.

Impact

An attacker can acquire much share of rewards by sandwiching transaction which transfers rewards to MlumStaking.

Code Snippet

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

Tool used

Manual Review

Recommendation

We have to add the lowest limit check of lockDuration as follows.

+   uint256 public _minLockDuration = 1 days;
    function createPosition(uint256 amount, uint256 lockDuration) external override nonReentrant {
        // no new lock can be set if the pool has been unlocked
        if (isUnlocked()) {
            require(lockDuration == 0, "locks disabled");
        }
+       else{
+           require(lockDuration >= _minLockDuration, "small duration");
+       }

        _updatePool();

        // handle tokens with transfer tax
        amount = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amount);
        require(amount != 0, "zero amount"); // createPosition: amount cannot be null

        // mint NFT position token
        uint256 currentTokenId = _mintNextTokenId(msg.sender);

        // calculate bonuses
        uint256 lockMultiplier = getMultiplierByLockDuration(lockDuration);
        uint256 amountWithMultiplier = amount * (lockMultiplier + 1e4) / 1e4;

        // create position
        _stakingPositions[currentTokenId] = StakingPosition({
            initialLockDuration: lockDuration,
            amount: amount,
            rewardDebt: amountWithMultiplier * (_accRewardsPerShare) / (PRECISION_FACTOR),
            lockDuration: lockDuration,
            startLockTime: _currentBlockTimestamp(),
            lockMultiplier: lockMultiplier,
            amountWithMultiplier: amountWithMultiplier,
            totalMultiplier: lockMultiplier
        });

        // update total lp supply
        _stakedSupply = _stakedSupply + amount;
        _stakedSupplyWithMultiplier = _stakedSupplyWithMultiplier + amountWithMultiplier;

        emit CreatePosition(currentTokenId, amount, lockDuration);
    }

Duplicate of #74