Lines of code

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

Vulnerability details

Impact

The current process for updating lock durations affects completed locks that are ready for unlocking When a user attempts to set a new lock duration while having tokens that have matured and ready for unlocking, the system may inadvertently prevent them from unlocking these tokens. as the lock duration will be updated.

An attempt to extend lock durations while having completed locks ready for unlocking will relock the matured tokens for the extended period, the system may erroneously consider these completed locks as active, potentially preventing users from accessing their unlocked tokens.

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

based on the code above if quantity is > 0 the function proceeds to update the duration, consider a scenario where a user has pending tokens to unlock but still calls this function. He ends up extending the duration even though they are ready to be unlocked.

POC

Consider Alice who has 100 tokens locked for "10" days, the tokens become available for unlocking after the 10th day. Alice now tries to set her lock duration to 15 days, the system will iterate the tokens alice holds and since quantity will be > 0 (lockedTokens[msg.sender][tokenContract].quantity > 0) the function will proceed and treat the completed lock (ready for unlock) as an active lock and proceeds to update its unlock time. As a result, Alice will now have to wait another 15 days to unlock the tokens again.

Tools Used

Manual Review

Recommended Mitigation Steps

add a check in the setLockDuration function not to include users tokens that have completed duration and are awaiting to be unlocked.

Assessed type

Invalid Validation

code-423n4 / 2024-05-munchables-findings