maushish - Missing `whenNotPaused`

[sherlock-admin4]

maushish

medium

Missing whenNotPaused

Summary

There is no whenNotPaused in restakeAfterLateExit due to which the user will still be able to stake even when Owner pauses the contract.

Vulnerability Detail

The good practice among all staking protocols where contracts are pausable is to:

• Stop the incoming stream of funds i.e is staking in the protocol.
• Allow all users to retrieve their funds if they want to.

  [!IMPORTANT] In emergency cases, if the above practice isn't followed, it could easily lead to various problems. Now in restakeAfterLateExit user naturally shouldn't be able to restake as _stake is using whenNotPaused but this could be easily bypassed by using an MEV bot watching for owner methods in mempool making protocol prone to different issues as per the emergency situation.

    /// @notice Allows a user to restake funds after exiting late by mistake.
    /// @dev Enforces restrictions on the new lock period based on the current lock period and default relock time.
    /// @param id The ID of the lock that was exited late and needs to be re-staked.
    /// @param typeIndex The new lock type index to apply for the restake.
    function restakeAfterLateExit(uint256 id, uint256 typeIndex) external {//audit: should be whenNotPaused
        // 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);

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

[!NOTE] The reason for this finding being a medium severity is that it breaks the core contract functionality of the owner modifier in emergency cases, Here is one similar finding to support my claim.

Impact

In cases where the owner wants to stop all activity, they still can't stop users from retaking their exited late flagged lock.

Code Snippet

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

Tool used

Manual Review

Recommendation

 /// @notice Allows a user to restake funds after exiting late by mistake.
    /// @dev Enforces restrictions on the new lock period based on the current lock period and default relock time.
    /// @param id The ID of the lock that was exited late and needs to be re-staked.
    /// @param typeIndex The new lock type index to apply for the restake.
-    function restakeAfterLateExit(uint256 id, uint256 typeIndex) external {
+   function restakeAfterLateExit(uint256 id, uint256 typeIndex) external whenNotPaused {

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

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

[santipu03]

This issue is invalid because the internal _stake function already has the pausable modifier.