krot-0025 - MlumStaking::emergencyWithdraw leads to loss in case of funds and not allows user to withdraw stake during emergency.

[sherlock-admin2]

krot-0025

Medium

MlumStaking::emergencyWithdraw leads to loss in case of funds and not allows user to withdraw stake during emergency.

Summary

When the user wants to withdraw during emergency i.e For e.g :- He is in need of funds and want to exit it will not allow because of position being locked and other is :- The user can call MlumStaking::withdrawFromPostion instead of MlumStaking::emergencyWithdraw because this call MlumStaking::withdrawFromPostion will allow them to take reward as well.

Vulnerability Detail

There will be a 2 cases or 2 scenarios which are as follows ::

Case 1

Assume there is an emergency from the protocol side like there is some fund loss in reward tokens or something other and they set the bool public _emergencyUnlock flag to true which will unlock all the position and allow anyone to withdraw their position from the staking. Now when this _emergencyUnlock flag is set true by the owner then it will allow user to call MlumStaking::emergencyWithdraw & MlumStaking::withdrawFromPostion Because there is no checks to prevent calling MlumStaking::withdrawFromPostion this function.

This will result in user using MlumStaking::withdrawFromPostion function to remove their position because this function will help them earn rewards instead of MlumStaking::emergencyWithdraw function because it only return the staked token from the position not the reward and this will result in the loss for the Protocol as it cause loosing out all the funds during emergency .

Case 2

Assume there is an emergency from the user side he is in a need of fund and he wants to close his position then MlumStaking::emergencyWithdraw will not work until the lock duration of the position being completed , So this function is of no use for the user during his emergency. Because of the lock duration it will not allow user to withdraw his stakes .

Let's see both scenarios with POC ::
For using this POC you have to add this in IMlumStaking.sol and have to add override in the MlumStaking::setEmergencyUnlock

    function setEmergencyUnlock(bool emergencyUnlock_) external; // @audit : Using for testing 

 function testEmergency() public {
        _rewardToken.mint(address(_pool), 1_000_000_000_000 );
        _stakingToken.mint(ALICE, 2 ether);

        console.log("Initial Balance of the Reward Token in the contract ", _rewardToken.balanceOf(address(_pool)));
        console.log("Initial Balance of Reward Token of Alice", _rewardToken.balanceOf(ALICE));
        console.log("Initial Balance of Alice of Staked Token", _stakingToken.balanceOf(ALICE));
        console.log("------------------------------------------------------------------------------------");
        console.log("------------------------------------------------------------------------------------");

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

        vm.prank(DEV);
        _pool.setEmergencyUnlock(true);
        skip(3 days);

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

        console.log("Balance of rewardToken in contract :-", _rewardToken.balanceOf(address(_pool)));
        console.log("Balance of Alice of Reward Token after Withdrawal", _rewardToken.balanceOf(ALICE));
        console.log("Balance of Alice of Staked Token", _stakingToken.balanceOf(ALICE));
    }

Here You can see the Output for the :-

Case 1

So, here during emergency the user is calling the MlumStaking::withdrawFromPosition and it will result in getting rewards also :-

And When the user use the MlumStaking::emergencyWithdraw he will be not able to get the rewards and only receive his staked amount back.

Impact

Loss of funds during the emergency for the Protocol

Code Snippet

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#L536-L560

Tool used

Manual Review

Foundry

Recommendation

For the first scenario to prevent user calling MlumStaking::withdrawFromPosition we have to add checks which will not allow user to call MlumStaking::withdrawFromPosition during emergency.

    function withdrawFromPosition(uint256 tokenId, uint256 amountToWithdraw) external nonReentrant {
         _requireOnlyApprovedOrOwnerOf(tokenId);
+        require(_emergencyUnlock == false, "Cannot use withdraw during emergency. You should use emergencyWithdraw function");
        _updatePool();
        address nftOwner = ERC721Upgradeable.ownerOf(tokenId);
        _withdrawFromPosition(nftOwner, tokenId, amountToWithdraw);
    }

[WangSecurity]

Invalid based on #290 (comment) and #290 (comment) comments.