Munchables - Stefanov's results

Lines of code

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

Vulnerability details

Impact

Users could unlock their tokens earlier

Proof of Concept

Function can be called at any time and uint32(block.timestamp) + uint32(_duration) can be way more than lastLockTime + uint32(_duration).

For example lastLockTime is set on Monday and the unlockTime is set to Sunday, that means 6 days lock duration. However, the function can be called 3 days after, on Thursday, to set the duration on 4 days (2 days less than initial 6 days). That if statement should look like this:

if (
    uint32(block.timestamp)/* (lastLockTime + 3 days) */ + uint32(_duration) /* 
    (4 days in our case) */ < lockedTokens[msg.sender][tokenContract].unlockTime 
    /* 6 days in our case */
) {
    revert LockDurationReducedError();
}

 uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
        .lastLockTime; 
lockedTokens[msg.sender][tokenContract].unlockTime =
     lastLockTime + 
     uint32(_duration); // 4 days

The If check will pass and the user is going to set the duration on 4 days, which is 2 days earlier.

Tools Used

Manual

Recommended Mitigation Steps

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
+               uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
+                   .lastLockTime;
-               if (
-                   uint32(block.timestamp) + uint32(_duration) <
-                   lockedTokens[msg.sender][tokenContract].unlockTime
-               ) {
+               if (
+                   lastLockTime + 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);
    }

Assessed type

Invalid Validation