Players can gain more NFTs benefiting from that past remainder in subsequent locks

mediumCode4rena

Lines of code

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

Vulnerability details

Description

The remaining locked tokens continue to accumulate after the player unlocks their tokens, whether partially or fully.

As a result, the player benefits by retrieving more NFTs when they lock their tokens again during the lock drop period or in the next lock drop period.

Location: LockManager::_lock()L344 - L380

solidity📋

    uint256 quantity = _quantity + lockedToken.remainder;
    uint256 remainder;
    uint256 numberNFTs;
    uint32 _lockDuration = playerSettings[_lockRecipient].lockDuration;

    // SNIPPED

    if (    
        lockdrop.start <= uint32(block.timestamp) &&
        lockdrop.end >= uint32(block.timestamp)
    ) {
        if (
            _lockDuration < lockdrop.minLockDuration ||
            _lockDuration >
            uint32(configStorage.getUint(StorageKey.MaxLockDuration))
        ) revert InvalidLockDurationError();
        if (msg.sender != address(migrationManager)) {
            // calculate number of nfts
-->         remainder = quantity % configuredToken.nftCost; 
            numberNFTs = (quantity - remainder) / configuredToken.nftCost;

            if (numberNFTs > type(uint16).max) revert TooManyNFTsError();

            // Tell nftOverlord that the player has new unopened Munchables
            nftOverlord.addReveal(_lockRecipient, uint16(numberNFTs));
        }
    }

  // SNIPPED 

->  lockedToken.remainder = remainder;
    lockedToken.quantity += _quantity;

Location: LockManager::unlock()L401 - L427

solidity📋
    function unlock(
        address _tokenContract,
        uint256 _quantity
    ) external notPaused nonReentrant {
        LockedToken storage lockedToken = lockedTokens[msg.sender][
            _tokenContract
        ];
        if (lockedToken.quantity < _quantity)
            revert InsufficientLockAmountError();
        if (lockedToken.unlockTime > uint32(block.timestamp))
            revert TokenStillLockedError();

        // force harvest to make sure that they get the schnibbles that they are entitled to
        accountManager.forceHarvest(msg.sender);

        lockedToken.quantity -= _quantity;

        // send token
        if (_tokenContract == address(0)) {
            payable(msg.sender).transfer(_quantity);
        } else {
            IERC20 token = IERC20(_tokenContract);
            token.transfer(msg.sender, _quantity);
        }

        emit Unlocked(msg.sender, _tokenContract, _quantity);
    }

Impact

During the lock drop period or in the subsequent lock drop period, the player retrieves more NFTs in proportion to the quantity locked. The remaining tokens from previous locks enhance the new locking quantity when relocked until the end of the lock period.

Moreover, it probably assumes that the lockDuration can either be equal or greater than the lock drop duration (lockDuration >= lockdrop.start - lockdrop.end), allowing players to reenter by each lock drop, or significantly less than the lock drop duration (lockDuration < lockdrop.start - lockdrop.end), enabling players to reenter during the same lockdrop.

Therefore, the likelihood of the issue is based on how offset players can reenter and take advantage of the remainders.

The following core functions take an effect of this issue:

• _lock()
• lockOnBehalf()
• lock()
• unlock()

Proof of Concept

Prerequisite

• The LockManager contract has zero balance.
• The actions taken during the any active lock drop period.
• Using the address(0) as a token contract represents locking the native token
  • _tokenContract = address(0)
• Assuming the configuration of nftCostof the address(0) is 3 ethers (3e18)
  • configuredTokens[_tokenContract: address(0)] = 3e18

---

Partially Unlocking Case

1. Lock 10e18 (_quantity) tokens as uint256 quantity = _quantity + lockedToken.remainder :: L344

The state updates as follows:

• quantity = 10e18(_quantity) + 0 (lockedToken.remainder) = 10e18
• remainder = 10e18 % 3e18 = 1e18
• numberNFTs = (10e18 - 1e18) / 3e18 = 3

Updates to lockedToken struct and LockManager contract balance:

• lockedToken.remainder = remainder = 1e18
• lockedToken.quantity += _quantity = 0 + 10e18 = 10e18
• address(LockManager).balance = 10e18

📍 This shows that locking of 10e18 tokens can retrieve 3 NFTs.

solidity📋
Location: LockManager.sol::_lock()

    // add remainder from any previous lock
--> L344:   uint256 quantity = _quantity + lockedToken.remainder;
            uint256 remainder;
            uint256 numberNFTs;
            uint32 _lockDuration = playerSettings[_lockRecipient].lockDuration;

    // SNIPPED

    if (    
        lockdrop.start <= uint32(block.timestamp) &&
        lockdrop.end >= uint32(block.timestamp)
    ) {
        if (
            _lockDuration < lockdrop.minLockDuration ||
            _lockDuration >
            uint32(configStorage.getUint(StorageKey.MaxLockDuration))
        ) revert InvalidLockDurationError();
        if (msg.sender != address(migrationManager)) {
            // calculate number of nfts
--> L363:   remainder = quantity % configuredToken.nftCost; 
            numberNFTs = (quantity - remainder) / configuredToken.nftCost;

            if (numberNFTs > type(uint16).max) revert TooManyNFTsError();

            // Tell nftOverlord that the player has new unopened Munchables
            nftOverlord.addReveal(_lockRecipient, uint16(numberNFTs));
        }
    }

  // SNIPPED 

-> L379 lockedToken.remainder = remainder;
        lockedToken.quantity += _quantity;

🕔 After the lockDuration has passed, the player can unlock the locked tokens.

2. Unlock 5e18 tokens partially:

• L416::lockedToken.quantity -= _quantity = 10e18 - 5e18 = 5e18
• address(LockManager).balance = 5e18

3. Lock 5e18 tokens back again as uint256 quantity = _quantity + lockedToken.remainder (prev remainder#1);

• quantity = 5e18(_quantity) + 1e18 (lockedToken.remainder) = 6e18
• remainder = 6e18 % 3e18 = 0
• numberNFTs = (6e18 - 0) / 3e18 = 2

Updates to lockedToken struct and LockManager contract balance:

• lockedToken.remainder = remainder = 0
• lockedToken.quantity += _quantity = 5e18 + 5e18 = 10e18
• address(LockManager).balance = 10e18

📍 In total, the player retrieves 5 NFTs (3 from the first locking + 2 from the second locking) by locking 10e18 tokens, benefiting from the remainder of the previous locking.

---

Fully Unlocking Case

---

1. Lock 13e18 (_quantity) tokens as uint256 quantity = _quantity + lockedToken.remainder :: L344

The state updates as follows:

• quantity = 13e18(_quantity) + 0 (lockedToken.remainder) = 10e18
• remainder = 13e18 % 3e18 = 1e18
• numberNFTs = (13e18 - 1e18) / 3e18 = 4 📍

Updates to lockedToken struct and LockManager contract balance:

• lockedToken.remainder = remainder = 1e18 📍
• lockedToken.quantity += _quantity = 0 + 13e18 = 13e18 📍
• address(LockManager).balance = 13e18 📍

📍 This shows that locking of 13e18 tokens can retrieve 4 NFTs.

🕔 After the lockDuration has passed, the player can unlock the locked tokens.

2. Unlock 13e18 tokens fully:

• L416::lockedToken.quantity -= _quantity = 13e18 - 13e18 = 0 📍
• address(LockManager).balance = 0 📍

3. Lock 11e18 tokens as uint256 quantity = _quantity + lockedToken.remainder (prev remainder#1);

• quantity = 11e18(_quantity) + 1e18 (lockedToken.remainder) = 12e18
• remainder = 12e18 % 3e18 = 0
• numberNFTs = (12e18 - 0) / 3e18 = 4 📍

Updates to lockedToken and LockManager contract balance:

• lockedToken.remainder = remainder = 0 📍
• lockedToken.quantity += _quantity = 0 + 11e18 = 11e18 📍
• address(LockManager).balance = 11e18 📍

📍 In total, the player retrieves 4 new NFTs from locking 11e18 tokens. Compared to the first locking of 13e18 tokens, the second lock yields similar power to gain 4 NFTs, benefiting from the remainder of the previous locking round.

Tools Used

• Manual Review

Recommended Mitigation Steps

Manage the remaining tokens when the player unlocks locked tokens. Alternatively, use other methods to handling the total amount of NFTs and locked quantity during the lock drop.

Assessed type

Math