sherlock-audit / 2024-05-gamma-staking-judging

9 stars 7 forks source link

aman - Add whenNotPause on `exitLateById` #289

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

aman

medium

Add whenNotPause on exitLateById

Summary

The User who has called the exitLateById would not be able to stake again because of whenNotPause modifier on stake function and no such modifier on exitLateById.

Vulnerability Detail

The Protocol allow stakers to un-stake either early or late via calling earlyExitById or exitLateById function respectively . So in case of early exit the staker will pay the penalty fee and withdraw there tokens. while in other case the staker could withdraw their token after cool down period or re-stake again via restakeAfterLateExit function. The Issue could arise when users wants to stake for other Multiplier or has called exitLateById this function mistakenly. In both cases The staker only need to select Multiplier which lock period is greater then either current lock period or default lock period. For Late exit user would call exitLateById:

function exitLateById(uint256 id) external {
        _updateReward(msg.sender); // Updates any pending rewards for the caller before proceeding.

        LockedBalance memory lockedBalance = locklist.getLockById(msg.sender, id); // Retrieves the lock details from the lock list as a storage reference to modify.

        // Calculate and set the new unlock time based on the remaining cooldown period.
        uint256 coolDownSecs = calcRemainUnlockPeriod(lockedBalance);
        locklist.updateUnlockTime(msg.sender, id, block.timestamp + coolDownSecs);

        // Reduce the locked supply and the user's locked balance with and without multiplier.
        uint256 multiplierBalance = lockedBalance.amount * lockedBalance.multiplier;
        lockedSupplyWithMultiplier -= multiplierBalance;
        lockedSupply -= lockedBalance.amount;
        Balances storage bal = balances[msg.sender];
        bal.lockedWithMultiplier -= multiplierBalance;
        bal.locked -= lockedBalance.amount;

        locklist.setExitedLateToTrue(msg.sender, id);

        _updateRewardDebt(msg.sender); // Recalculates reward debt after changing the locked balance.

        emit ExitLateById(id, msg.sender, lockedBalance.amount); // Emits an event logging the details of the late exit.
    }

and for restaking

function restakeAfterLateExit(uint256 id, uint256 typeIndex) external {
        // Retrieve the lock details for the specified ID.
        LockedBalance memory lockedBalance = locklist.getLockById(msg.sender, id);
        require(lockedBalance.exitedLate, "This lock was not exited late or is ineligible for restaking.");

        uint256 newLockPeriod = lockPeriod[typeIndex]; // Get the new lock period based on the type index.
        uint256 currentLockPeriod = lockedBalance.lockPeriod;

        // Enforce that the new lock period must be valid based on the current conditions.
        if (currentLockPeriod <= defaultRelockTime || (block.timestamp - lockedBalance.lockTime) < currentLockPeriod) {
            require(newLockPeriod >= currentLockPeriod, "New lock period must be greater than or equal to the current lock period");
        } else {
            require(newLockPeriod >= defaultRelockTime, "New lock period must be greater than or equal to the default relock time");
        }

        // Proceed to restake the funds using the new lock type.
  @----->      _stake(lockedBalance.amount, msg.sender, typeIndex, true); // @audit : this function has whenNotPause modifier

        // Remove the old lock record to prevent any further operations on it.
        locklist.removeFromList(msg.sender, id);

        emit RestakedAfterLateExit(msg.sender, id, lockedBalance.amount, typeIndex);
    }

The following case would occur:

  1. Bob stake 10 gamma tokens for 30 days with Multiplier 1.
  2. On 25th day Bob decided to stake for 60 days with Multiplier 2.
  3. Assume the staking is pause at this stage.
  4. Bob calls exitLateById from current Multiplier which is 1 in our case.
  5. The Rewards will stop accuring for bob now.
  6. Bob calls restakeAfterLateExit with index of Multiplier 2.
  7. the call will be reverted with because the staking is pause.

The above scenario could also occur mistakenly. as @notice tag explain on top of exitLateById

Test Case :

function testRestakePauseReverts() external {
        vm.prank(user1);
        lock.stake(100e18, user1, 0);
        vm.prank(address(lock.owner()));
        lock.pause();
        vm.prank(user1);
        lock.exitLateById(0);
        vm.prank(user1);
        lock.restakeAfterLateExit(0, 1); // Restake with the diff type index
    }

Place the test case inside RestakeAfterLateExit.t.sol and run with command : forge test --mt testRestakePauseReverts -vvv

Impact

The Staker could not be able to re-stake in case of Protocol is Pause.

Code Snippet

https://github.com/sherlock-audit/2024-05-gamma-staking/blob/main/StakingV2/src/Lock.sol#L349 https://github.com/sherlock-audit/2024-05-gamma-staking/blob/main/StakingV2/src/Lock.sol#L378

Tool used

Manual Review

Recommendation

Either add whenNotPause on exitLateById or allow the only the restaking while the protocol is paused.

santipu03 commented 4 months ago

The protocol decided to make pausable entry-point functions (stake and restake) and not some of the exit-point functions (exiting late and withdrawals). In case of an emergency, users will be able to withdraw the staked funds but not to stake again or withdraw rewards. Because this is the intended design of the protocol, this issue is low (invalid).