Munchables - steadyman's results

Lines of code

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

Vulnerability details

Impact

The logic check in the setLockDuration function of the LockManager contract has a problem, which allows the lock duration to be decreased.

Proof of Concept

In the setLockDuration function's logic check, there is a verification for the new _duration, requiring uint32(block.timestamp) + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime. However, the new unlockTime is set to lastLockTime + uint32(_duration). This results in the new unlockTime being less than the old unlockTime.

Example: Bob locks tokens with a duration of 10 days, making the unlockTime the 11th day. On the 4th day, Bob calls the setLockDuration function with _duration set to 7 days. This check passes, and the new unlockTime becomes the 8th day, thus reducing the lock duration.

Tools Used

Manual Audit

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) {
+                uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
+                    .lastLockTime;
                // check they are not setting lock time before current unlocktime
                  if (
+                    uint32(lastLockTime) + uint32(_duration) <
+                    lockedTokens[msg.sender][tokenContract].unlockTime
+                ) {
+                    revert LockDurationReducedError();
+                }

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

Assessed type

Other