karsar - extendLockPosition function May Not Perform as Expected

[sherlock-admin2]

karsar

High

extendLockPosition function May Not Perform as Expected

Summary

When a user attempts to extend their lock position , the _lockPosition function causes them to lose their waited time from previous lock.

Vulnerability Detail

_lockPostion is used to extend or renew its lock position. The issue lies in extendLockPosition this function is used to extend its lock position , the lock position shouldn't be expired , should be greater than the current lock duration and isUnlocked() == false. When a user extends its position the startLockTime is set to currentBlockTimestamp it means that user time will reset
POC:

1. alice creates a position , LockDuration = 60 days.
2. Owner locks now its possible to LockPosition.
3. a voting period has started which need 90 days to vote.
4. skip(50 days).
5. alice calls extendLockPosition(1 , 90 days ).
6. added time 90 - 60 days i.e 30 days .
7. then startLockTime is set to currentBlockTimestamp.position.startLockTime = currentBlockTimestamp;
8. so then alice time reset to 1.
9. alice needs to wait whole 90 days losing her 50 days waited time.

File: MlumStaking.sol
836:     function _lockPosition(
837:         uint256 tokenId,
838:         uint256 lockDuration,
839:         bool resetInitial
840:     ) internal {
841:         require(!isUnlocked(), "locks disabled");
842: 
843:         StakingPosition storage position = _stakingPositions[tokenId];
844: 
845:         // for renew only, check if new lockDuration is at least = to the remaining active duration
846:         uint256 endTime = position.startLockTime + position.lockDuration;
847:         uint256 currentBlockTimestamp = _currentBlockTimestamp();
848:         if (endTime > currentBlockTimestamp) {
849:             require(
850:                 lockDuration >= (endTime - currentBlockTimestamp) &&
851:                     lockDuration > 0,
852:                 "invalid"
853:             );
854:         }
855: 
856:         // for extend lock postion we reset the initial lock duration
857:         // we have to check that the lock duration is greater then the current
858:         if (resetInitial) {
859:             require(lockDuration > position.initialLockDuration, "invalid");
860:             position.initialLockDuration = lockDuration;
861:         }
862: 
863:         _harvestPosition(tokenId, msg.sender);
864: 
865:         position.lockDuration = lockDuration;
866:         position.lockMultiplier = getMultiplierByLockDuration(lockDuration);
867:         position.startLockTime = currentBlockTimestamp;
868:         _updateBoostMultiplierInfoAndRewardDebt(position);
869: 
870:         emit LockPosition(tokenId, lockDuration);
871:     }

Impact

lost lock time of users.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L691-L718

Tool used

Manual Review

Recommendation

Now when extending LockDuration user's initialLockDuration is set to position startLockTime and its starts from its startLockTime period.

File: MlumStaking.sol
   function _lockPosition(
        uint256 tokenId,
        uint256 lockDuration,
        bool resetInitial
    ) internal {
        require(!isUnlocked(), "locks disabled");

        StakingPosition storage position = _stakingPositions[tokenId];

        // for renew only, check if new lockDuration is at least = to the remaining active duration
        uint256 endTime = position.startLockTime + position.lockDuration;
        uint256 currentBlockTimestamp = _currentBlockTimestamp();
        if (endTime > currentBlockTimestamp) {
            require(
                lockDuration >= (endTime - currentBlockTimestamp) &&
                    lockDuration > 0,
                "invalid"
            );
        }

        // for extend lock postion we reset the initial lock duration
        // we have to check that the lock duration is greater then the current
-        if (resetInitial) {
-           require(lockDuration > position.initialLockDuration, "invalid");
-           position.initialLockDuration = lockDuration;
-        }

        _harvestPosition(tokenId, msg.sender);

        position.lockDuration = lockDuration;
-        position.lockMultiplier = getMultiplierByLockDuration(lockDuration);
        position.startLockTime = currentBlockTimestamp;
-        _updateBoostMultiplierInfoAndRewardDebt(position);
+       if (resetInitial) {
+           require(lockDuration > position.initialLockDuration, "invalid");
+           position.initialLockDuration = lockDuration;
+           position.startLockTime = position.startLockTime;
+        }

+        position.lockMultiplier = getMultiplierByLockDuration(lockDuration);
+         _updateBoostMultiplierInfoAndRewardDebt(position);

        emit LockPosition(tokenId, lockDuration);
    }

Duplicate of #138