Munchables - 0xmystery's results

Lines of code

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

Vulnerability details

Impact

The issue in the setLockDuration function allows users to set an unlockTime that is inconsistent with the intended lock duration. By basing the unlockTime on a practically outdated lastLockTime, the function enables users to manipulate their lock duration. This manipulation can undermine the contract's rules for token locking, leading to exploit that allows users to unlock their tokens earlier than intended or set a lock duration that does not align with the current time, impacting the fairness and integrity of the locking mechanism.

Proof of Concept

Here's the affected code in setLockDuration():

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

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

And, a typical exploit scenario could be as follows:

• Suppose lastLockTime is far in the past (e.g., several weeks ago).
• By calling setLockDuration with a specific _duration, the user can set an unlockTime that does not reflect the current block timestamp.
• This allows the user to unlock their tokens immediately or much sooner than the intended lock duration.

Detailed steps:

• Assume the current block timestamp is 100000.
• The lastLockTime for a user's token is 50000.
• The existing unlockTime is 150000.
• The user calls setLockDuration with _duration of 50000. (100000 + 50000) == 150000 skip the following revert in setLockDuration().

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

                // check they are not setting lock time before current unlocktime
                if (
                    uint32(block.timestamp) + uint32(_duration) <
                    lockedTokens[msg.sender][tokenContract].unlockTime
                ) {
                    revert LockDurationReducedError();
                }

• The new unlockTime is set to 50000 + 50000 = 100000.
• Given the current timestamp is 100000, the unlockTime of 100000 could now skip the following revert in unlock().

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

        if (lockedToken.unlockTime > uint32(block.timestamp))
            revert TokenStillLockedError();

• This means the user can unlock their tokens immediately instead of waiting until the intended unlockTime of 150000.

Tools Used

Manual

Recommended Mitigation Steps

Ensure the unlockTime is calculated based on the current block timestamp to prevent manipulation of the lock duration.

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

            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(block.timestamp) +
                    uint32(_duration);
            }

Assessed type

Context