deepkin - [High] Token.setReservedUntilTokenId() do not recalculate existing founder's vesting allocation ids.

[sherlock-admin2]

deepkin

high

[High] Token.setReservedUntilTokenId() do not recalculate existing founder's vesting allocation ids.

Summary

Token contract have been updated with new feature Token Reserve. This change includes new storage variable reservedUntilTokenId, new setter setReservedUntilTokenId() and a lot of other logic reworks to handle reservedUntilTokenId.

Vulnerability Detail

When setReservedUntilTokenId() called there is no logic to update existing founder's vesting allocation ids.

Steps to reproduce:

1. Token initialized with reservedUntilTokenId = 10 and founderList.
2. Founders configured during initialization and their tokens calculated and added with _addFounders()
3. During step 2 founder tokens will be calculated based on current reservedUntilTokenId.
   tokenRecipient[10] = founderAddress

                // Used to store the base token id the founder will recieve
                uint256 baseTokenId = reservedUntilTokenId;

                // For each token to vest:
                for (uint256 j; j < founderPct; ++j) {
                    // Get the available token id
                    baseTokenId = _getNextTokenId(baseTokenId);

                    // Store the founder as the recipient
                    tokenRecipient[baseTokenId] = newFounder;

                    emit MintScheduled(baseTokenId, founderId, newFounder);

                    // Update the base token id
                    baseTokenId = (baseTokenId + schedule) % 100;
                }
            }

4. setReservedUntilTokenId() called with new value that bigger than previous newReservedUntilTokenId = 20.
5. Minter mint 15 tokens with mintFromReserveTo(). It will work because current reservedUntilTokenId = 20 and _mint() have no protection.

    function mintFromReserveTo(address recipient, uint256 tokenId) external nonReentrant onlyMinter {
        // Token must be reserved
        if (tokenId >= reservedUntilTokenId) revert TOKEN_NOT_RESERVED();

        // Mint the token without vesting (reserved tokens do not count towards founders vesting)
        _mint(recipient, tokenId);
    }

6. Result: All founder's tokens with id less then current reservedUntilTokenId = 20 will be minted for another users.

Impact

This can can lead to inconsistency between founder's expected token in tokenRecipient[] and reservedUntilTokenId in result founder's token can be simply minted for another user as reserved with mintFromReserveTo().

Severity justification:
Marked it as High severity issue because Token Reserve is a new core functionality of this update and the impact will cause founder to lose his vesting tokens.

Code Snippet

setReservedUntilTokenId()

    function setReservedUntilTokenId(uint256 newReservedUntilTokenId) external onlyOwner {
        // Cannot change the reserve after any non reserved tokens have been minted
        // Added to prevent making any tokens inaccessible
        if (settings.mintCount > 0) {
            revert CANNOT_CHANGE_RESERVE();
        }

        // Cannot decrease the reserve if any tokens have been minted
        // Added to prevent collisions with tokens being auctioned / vested
        if (settings.totalSupply > 0 && reservedUntilTokenId > newReservedUntilTokenId) {
            revert CANNOT_DECREASE_RESERVE();
        }

        // Set the new reserve
        reservedUntilTokenId = newReservedUntilTokenId;

        emit ReservedUntilTokenIDUpdated(newReservedUntilTokenId);
 }

Tool used

Manual Review

Recommendation

Make updateFounders() public and call it during the setReservedUntilTokenId().