Munchables - Janio's results

Lines of code

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

Vulnerability details

Users can decrease lock duration and receive Schnibbles before expected time, impacting NFT minting and economics of the game

User will be able to claim schnibbles before minLockDuration will allow user to level his munchable and sell the NFT before expected, devaluing the game market

Proof of Concept

The function LockManager.setLockDuration can be called to update user's locking period. The new locking period must be greater than the current unlockTime. However, the user can reduce the unlockTime value when calling the function LockManager.setLockDuration by making uint32(block.timestamp) + uint32(_duration) equals or greater to unlockTime, manipulating the value of _duration accordingly in LockManager.sol#L256:

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

The problem is to set the new unlockTime without checking if it is not earlier than the current period: LockManager.sol#L265

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

depending how much lastLockTime is old, the user will unlock his assets and receive schnibbbles earlier than expected.

The users can call setLockDuration multiple times to lower the unlockTime as much low as like one month earlier than before. The limit would be as low as 1 day.

The user can repeat this algorithm: 1 - lock his ether and get corresponding NFTs, by calling lock or lockOnBehalf 2 - call setLockDuration to lower the unlockTime 3 - if the duration is the lowest (usually 1 day), he will unlock his ether (or tokens) and claim schnibbles before minLockDuration time 4 - if the duration is still longer, he goes to step two again, and call setLockDuration, lowering the duration again

Note that the user can repeat this process again after one day: lock again, call setLockDuration, since he claimed back his assets.

By calling setLockDuration multiple times, he can lower the unlockTime, call unlock and claim schnibbles before minLockDuration, as low as one day. The schnibbles will allow user level up and sell the munchable NFT before expected, lowering the value market of the game.

One would be tempted to think this issue could be softened because the LockManager.lock function checks if _lockDuration < lockdrop.minLockDuration. But the issue is still severe, because the user can lock first, then call setLockDuration many times and lower _lockDuration. With his assets back, he can repeat this process, greatly profting from the game, without respecting the minimum duration lock period.

POC Example

lets say timestamp is today (2024-05-27) : 1716836368
Considering that the last unlocktime was 30 days ago (1713166814) and previous unlock time is 90 days from now (1723707614 or 2024-08-15)

setting the duration to 60 days, for example

uint32(block.timestamp) + uint32(_duration) = 1721029214 unlocktime = 1723707614

the condition (uint32(block.timestamp) + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime) pass.

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

las unlock time: 1713166814 new unlock time will be: 1718437214 (2024-06-15) before the previous unlock time. Then setting it smaller.

Tools Used

IntelliJ IDEA

Recommended Mitigation Steps

Guarantee that the new unlockTime is bigger than the previous set lock time. Also make sure that the unlockTime is bigger than the minimum lock duration period.

First, check if the new unlockTime will be bigger than the previous unlockTime:

LockManager.sol#L263

    uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
                    .lastLockTime;

+    uint32 lastUnlockTime = lockedTokens[msg.sender][tokenContract].unlockTime;
+    if((lastLockTime + uint32(_duration)) < lastUnlockTime) revert InvalidUnlockTime();

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

Add the equals condition to the below check: LockManager.sol#L265

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

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

Assessed type

Invalid Validation