A user is capable of reducing the unlockTime of his locked token

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L245-L272

Vulnerability details

Impact

A user can reduce his lockedToken.unlockTime by using the setLockDuration function

Proof of Concept

1. For simplicity lets say that Bob has locked some tokens and now his lockedTokens[msg.sender][tokenContract].lastLockTime; is day 0 and lockedTokens[msg.sender][tokenContract].unlockTime is day 30.
2. After 10 days he calls the setLockDuration function with _duration = 21 days.
3. This if statement will pass if ( uint32(block.timestamp) + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime )
4. His lockedTokens[msg.sender][tokenContract].unlockTime will be set to lastLockTime + uint32(_duration);, which is equal to day 0 + day 21 = 21 days.

5. Bob has just shortened his lockedTokens[msg.sender][tokenContract].unlockTime by 9 days.

   function setLockDuration(uint256 _duration) external notPaused {
       if (_duration > configStorage.getUint(StorageKey.MaxLockDuration))
           revert MaximumLockDurationError();
   
       playerSettings[msg.sender].lockDuration = uint32(_duration);
       // update any existing lock
       uint256 configuredTokensLength = configuredTokenContracts.length;
       for (uint256 i; i < configuredTokensLength; i++) {
           address tokenContract = configuredTokenContracts[i];
           if (lockedTokens[msg.sender][tokenContract].quantity > 0) {
               // check they are not setting lock time before current unlocktime
               if (
                   uint32(block.timestamp) + uint32(_duration) <
                   lockedTokens[msg.sender][tokenContract].unlockTime
               ) {
                   revert LockDurationReducedError();
               }
   
               uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
                   .lastLockTime;
               lockedTokens[msg.sender][tokenContract].unlockTime =
                   lastLockTime +
                   uint32(_duration);
           }
       }
   
       emit LockDuration(msg.sender, _duration);
   }

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of setting the lockedTokens[msg.sender][tokenContract].unlockTime to lastLockTime + uint32(_duration);, it should instead be set to uint32(block.timestamp) + uint32(_duration).

Assessed type

Math

[c4-judge]

alex-ppg marked the issue as duplicate of #89

[c4-judge]

alex-ppg marked the issue as partial-75