utsav - Rewards can be stolen in MlumStaking.sol by frontrunning

[sherlock-admin2]

utsav

Medium

Rewards can be stolen in MlumStaking.sol by frontrunning

Summary

Rewards can be stolen in MlumStaking.sol by frontrunning

Vulnerability Detail

MlumStaking receives rewardToken and distribute it among the stakers, who are staking. An attacker can frontrun the rewardToken deposit to receive rewards and withdraw immediately after receiving rewards, successfully stealing other users reward

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

function withdrawFromPosition(uint256 tokenId, uint256 amountToWithdraw) external nonReentrant {
        _requireOnlyApprovedOrOwnerOf(tokenId);

        _updatePool();
        address nftOwner = ERC721Upgradeable.ownerOf(tokenId);
        _withdrawFromPosition(nftOwner, tokenId, amountToWithdraw);
    }

Attacker will create position that has 0 lock duration before the rewardToken deposit and due to 0 lock duration attacker will be able to withdraw immediately after staking along with rewards

All this happening due to _accRewardsPerShare which increases due to increase in rewardToken, so everytime there is rewardToken deposited, it increases the _accRewardsPerShare and an attacker can take advantage of that

Impact

Rewards will be stolen

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L354C4-L391C1
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L574C4-L592C1
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L496C5-L503C1

Tool used

VS code

Recommendation

Add some delay when a user creates position or use time based calculation

Duplicate of #74