Skip to content
This repository was archived by the owner on Jan 12, 2025. It is now read-only.

Praise03 - During Emergency Unlock, User can call withdrawFromPosition to make premature withdrawals and get full rewards. #575

Closed
sherlock-admin4 opened this issue Jul 15, 2024 · 1 comment
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Jul 15, 2024

Praise03

Medium

During Emergency Unlock, User can call withdrawFromPosition to make premature withdrawals and get full rewards.

Impact

Contract loses reward tokens unfairly and user can gain reward tokens without waiting the necessary lock duration

Code Snippet

function testGainRewardsBeforeDurationPasses() public {
        _stakingToken.mint(ALICE, 1 ether);

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

        _rewardToken.mint(address(_pool), 100_000_000);

        console.log(_pool.pendingRewards(1));
        //99999999; should be unlocked after lock duration passes

        //Contract is put into emergency mode and all withdrawals are unlocked
        vm.prank(DEV);
        _pool.setEmergencyUnlock(true);

        // alice withdraws her position
        vm.prank(ALICE);
        // _pool.harvestPosition(1);
        _pool.withdrawFromPosition(1, 1 ether);

        //returns 99999999
        console.log(_rewardToken.balanceOf(ALICE));

    }

In the above POC it is demonstrated that users can gain reward tokens before their lock duration elapses in the event of an emergency unlock. The protol incorrectly assumes that the users will call the _emergencyWithdraw function during and emergency which would mean their position gets closed and they get no rewards.
However, users can call the withdrawFromPosition (or harvestPosition) and close their position prematurely and also get rewards.

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L626

require(
            _unlockOperators.contains(nftOwner)
                || (position.startLockTime + position.lockDuration) <= _currentBlockTimestamp() || isUnlocked(),
            "locked"
        );

The isUnlocked check above allows the call to go through during emergency unlocks

A malicious user could also intentionally frontrun the owners call to put the contract into emergency mode with a stakingToken deposit and then withdraw the deposited amount + rewards in the next block.

Tool used

Manual Review

Recommendation

Consider restricting the withdrawFromPosition and harvestPosition functons from being called during emergency unlocks

@github-actions github-actions bot added duplicate Medium A Medium severity issue. labels Jul 21, 2024
@sherlock-admin2 sherlock-admin2 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Fierce Mauve Robin - During Emergency Unlock, User can call withdrawFromPosition to make premature withdrawals and get full rewards. Praise03 - During Emergency Unlock, User can call withdrawFromPosition to make premature withdrawals and get full rewards. Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
@WangSecurity WangSecurity removed Medium A Medium severity issue. Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Aug 13, 2024
@sherlock-admin2 sherlock-admin2 added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue labels Aug 13, 2024
@WangSecurity
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

3 participants