araj - Malicious user can frontrun the reward deposit to MlumStaking.sol to steal rewards of honest stakers

[sherlock-admin3]

araj

Medium

Malicious user can frontrun the reward deposit to MlumStaking.sol to steal rewards of honest stakers

Summary

Malicious user can frontrun the reward deposit to MlumStaking.sol to steal rewards of honest stakers

Vulnerability Detail

User gets reward in MlumStaking.sol for staking token, but the problem is a malicious user can frontrun the reward deposit to MlumStaking.sol by creating a position with 0 lockDuration and withdrawing immediately after the rewardDeposit, successfully stealing rewards of honest stakers

//How this works(simple example)

1. Suppose there is a reward of 10e18(rewardToken) going to be deposited in MlumStaking for distribution
2. Malicious user saw this and frontrun to create a position of 2e18(stakedToken) with 0 lockDuration
3. Which means amountWithMultiplier will be = 2e18(getMultiplierByLockDuration will return 0 for 0 lockDuration) & rewardDebt will be amountWithMultiplier * _accRewardsPerShare / PRECISION_FACTOR( 2e18 * 1000(suppose) / PRECISION_FACTOR)

 function getMultiplierByLockDuration(uint256 lockDuration) public view returns (uint256) {
...
        if (_maxLockDuration == 0 || lockDuration == 0) return 0;
...
    }

  function createPosition(uint256 amount, uint256 lockDuration) external override nonReentrant {
...
        // calculate bonuses
        uint256 lockMultiplier = getMultiplierByLockDuration(lockDuration);
        uint256 amountWithMultiplier = amount * (lockMultiplier + 1e4) / 1e4;
...
        rewardDebt: amountWithMultiplier * (_accRewardsPerShare) / (PRECISION_FACTOR),
...
    }

4. Now rewardToken = 10e18 deposited in MlumStaking for distribution
5. Malicious user calls withdrawFromPosition() which calls _updatePool() that increases the _accRewardsPerShare due to increase in rewardToken. Lets say it became 1100(In pt-3 we assume it to be 1000)

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

    }

6. After updating pool, withdrawFromPosition() calls _harvestPosition() which calculates the pendingAmount and transfers to user, so pending = (2e18 * 1100 / PRECISION_FACTOR) - (2e18 * 1000 / PRECISION_FACTOR)

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;

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

7. Above pending will be some positive value due to increase in _accRewardsPerShare & it will be transfered to malicious user, successfully stealing rewards of stakers staking for months

Note: This pendingAmount can be very high, if amountWithMultiplier & rewardToken is high enough

Impact

Malicious user can steal rewards

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L354C5-L390C6
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L496C5-L502C6
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L574C4-L591C6
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L674C5-L687C1

Tool used

Manual Review

Recommendation

1. Stop creating positions with 0 lockDuration
2. Use time based calculation for reward instead of _accRewardsPerShare

[0xAraj]

Escalate

This is an duplicate of issue #74 which clearly shows how an attacker can frontrun to steal rewards

[sherlock-admin3]

Escalate

This is an duplicate of issue #74 which clearly shows how an attacker can frontrun to steal rewards

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.