Matin - The emergencyWithdraw() lacks the harvest function, leading to loss of rewards

[sherlock-admin3]

Matin

Medium

The emergencyWithdraw() lacks the harvest function, leading to loss of rewards

Summary

The absence of the harvesting mechanism in emergencyWithdraw() can lead to significant loss of accumulated rewards for users who invoke this function. This impacts users' earnings and diminishes the overall attractiveness of the staking mechanism, as users cannot retrieve the rewards they have earned up to the emergency withdrawal point.

Vulnerability Detail

The emergencyWithdraw() function in the contract MlumStaking is designed to allow stakers to withdraw their staked tokens during emergencies.
However, it does not call the _harvestPosition() function before withdrawing the tokens. This means that any accrued rewards up to that point are not claimed and are consequently lost.
As we can see in the contract, all the important functions responsible for depositing and withdrawing staked tokens call the _harvestPosition() to accrue the reward tokens.
The emergencyWithdraw() function, at its heart destroys the position (burns the corresponding NFT) and ignores all the accumulated rewards:

    function emergencyWithdraw(uint256 tokenId) external override nonReentrant {
        _requireOnlyOwnerOf(tokenId);

        StakingPosition storage position = _stakingPositions[tokenId];

        // position should be unlocked
        require(
            _unlockOperators.contains(msg.sender)
                || (position.startLockTime + position.lockDuration) <= _currentBlockTimestamp() || isUnlocked(),
            "locked"
        );
        // emergencyWithdraw: locked

        uint256 amount = position.amount;

        // update total lp supply
        _stakedSupply = _stakedSupply - amount;
        _stakedSupplyWithMultiplier = _stakedSupplyWithMultiplier - position.amountWithMultiplier;

        // destroy position (ignore boost points)
        _destroyPosition(tokenId);

        emit EmergencyWithdraw(tokenId, amount);
        stakedToken.safeTransfer(msg.sender, amount);
    }

The lack of harvesting the accumulated rewards would lead to loss of the accumulated reward tokens inside the contract 'MlumStaking`.

Impact

Loss of reward tokens during emergency withdrawing

Proof of Concept

Alice has been staking her tokens in a reward-generating pool for several weeks. During this period, she accumulates significant rewards due to the pool's incentive structure.

1. Alice stakes 100 tokens.
2. Over 4 weeks, she accumulates 20 reward tokens.
3. An unforeseen event causes Alice to urgently need her staked tokens. She wrongly calls the emergencyWithdraw() function to retrieve her 100 tokens.
4. The emergencyWithdraw() function withdraws her staked tokens but does not call the harvest function.
5. As a result, the 20 accumulated reward tokens remain unclaimed and are lost.

Alice loses the 20 reward tokens she earned over 4 weeks, impacting her total earnings and reducing the benefits of her participation in the staking pool.

Code Snippet

The Function emergencyWithdraw()

Tool used

Manual Review

Recommendation

Consider calling the _harvestPosition() before destroying the position inside the function emergencyWithdraw() in MlumStaking.

[WangSecurity]

Invalid based on the discussion under #460