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

9 stars 7 forks source link

befree3x - Missing size check of `typeIndex` parameter on `Lock::restakeAfterLateExit` could disallow a user from restaking funds after existing late by mistake #186

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

befree3x

medium

Missing size check of typeIndex parameter on Lock::restakeAfterLateExit could disallow a user from restaking funds after existing late by mistake

Summary

Missing size check of typeIndex parameter on Lock::restakeAfterLateExit could disallow a user from restaking funds after existing late by mistake

Vulnerability Detail

The function Lock::restakeAfterLateExit enables a user to restake funds after existing late by mistake. However, there is no safeguard on the value of typeIndex. If the later is bigger than the length of the array lockPeriod, it could lead to array out-of-bounds access panic, hence, disallowing the user from finishing the restaking action.

Link to the affected line of code: https://github.com/sherlock-audit/2024-05-gamma-staking/blob/main/StakingV2/src/Lock.sol#L382

Impact

A user providing a wrong lock type index could see his restaking action failed, e.g. not be able to restakeAfterLateExit if he weren't aware of the wrong value put in the typeIndex parameter.

Code Snippet

Scenario:

Code ```javascript function testUserACantRestakeAfterLateExitWithTooBigTypeIndex() public { // User A stakes initially vm.prank(user1); lock.stake(100e18, user1, 0); // User A stakes with the first lock type uint256 initialLockId = 0; // Assuming lockId starts at 0 and increments // Advance time to simulate a late exit scenario vm.warp(block.timestamp + 15 days); // Simulate passing 15 days vm.prank(user1); lock.exitLateById(initialLockId); // Fetch the lock details after late exit LockedBalance memory lockedBalanceAfterExit = lock.locklist().getLockById(user1, initialLockId); console.log("exitedLate flag: ", lockedBalanceAfterExit.exitedLate); assertTrue(lockedBalanceAfterExit.exitedLate, "exitLateById should have changed exitedLate to true"); // Now User A wants to restake the same amount for a new lock period vm.prank(user1); lock.restakeAfterLateExit(initialLockId, 3); // NOTE: the typeIndex value should be 0, instead of 3 } ```

Tool used

Manual review.

Recommendation

Consider adding the following modifications:

  1. In the file ILock.sol, add the new error definition:

+   error LockTypeIndexBiggerThanLockPeriodLength();

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.");
+       if (typeIndex >= lockPeriod.length) revert LockTypeIndexBiggerThanLockPeriodLength();
        uint256 newLockPeriod = lockPeriod[typeIndex]; 
...
}
santipu03 commented 4 months ago

This issue is invalid because it's based on a user mistake and it doesn't lead to a loss of funds.

User input validation: User input validation to prevent user mistakes is not considered a valid issue.