Munchables - gajiknownnothing's results

Lines of code

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

Vulnerability details

Impact

The setLockDuration() function allows users to change the lock duration of their tokens. However, due to a flawed check, users can set a new lock duration that is shorter than the original, effectively reducing their lock time.

Proof of Concept

When users lock tokens, the unlock time is set as block.timestamp + _lockDuration, _lock()#L382:

        lockedToken.unlockTime =
            uint32(block.timestamp) +
            uint32(_lockDuration);

The intended purpose is to prevent users from unlocking tokens before the specified unlock time. However, the check in setLockDuration() permits users to set a new duration that shortens the lock period, setLockDuration()#256:

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

This allows users to effectively bypass the lock period restriction. For example, if Alice locks tokens for 7 days and then, after 4 days, changes the lock duration to 3 days, the check will pass and Alice can unlock her tokens earlier than intended.

Tools Used

vscode

Recommended Mitigation Steps

                if (
                    lockedTokens[msg.sender][tokenContract].lastLockTime + uint32(_duration) <=
                    lockedTokens[msg.sender][tokenContract].unlockTime
                ) {
                    revert LockDurationReducedError();
                }

Assessed type

Invalid Validation