ast3ros - Founder loses first NFT due to flawed token ID calculation beyond 100 reserved tokens

[sherlock-admin2]

ast3ros

high

Founder loses first NFT due to flawed token ID calculation beyond 100 reserved tokens

Summary

A calculation error in determining the baseTokenID can lead to a founder not receiving their first NFT when the number of reserved tokens exceeds 100.

Vulnerability Detail

The DAO's addFounder function is designed to allocate NFTs to founders based on a reserved mapping of token indices (0-100). Founders are meant to receive NFTs within this range.

The issue arises when setting the first token ID for founders (reservedUntilTokenId). If this value is over 100, the calculation incorrectly assigns the founder's token ID outside the intended 0-100 range. This misalignment causes the founder to loss their first NFT.

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

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L160-L176

For example, with reservedUntilTokenId set to 120, meaning 120 tokens are reserved. The new baseTokenId to assign to the founder is calculated in _getNextTokenId.

    baseTokenId = _getNextTokenId(baseTokenId);

Let's inpsect the function _getNextTokenId, because there is no recipient assigned to tokenId 120 yet, the statement tokenRecipient[_tokenId].wallet != address(0) equals to false. The function _getNextTokenId returns _tokenId which is 120.

    /// @dev Finds the next available base token id for a founder
    /// @param _tokenId The ERC-721 token id
    function _getNextTokenId(uint256 _tokenId) internal view returns (uint256) {
        unchecked {
            while (tokenRecipient[_tokenId].wallet != address(0)) {
                _tokenId = (++_tokenId) % 100;
            }

            return _tokenId;
        }
    }

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L184-L194

It means the first NFT of the newFounder is assigned to token id 120, which is outside the intended range

When claiming the token, the _mintWithVesting function fails to recognize the founder's claim to token ID 120, as it is converted to baseTokenID 20 (uint256 baseTokenId = _tokenId % 100), which is already assigned to a different recipient.

    /// @dev Checks if a given token is for a founder and mints accordingly
    /// @param _tokenId The ERC-721 token id
    function _isForFounder(uint256 _tokenId) private returns (bool) {
        // Get the base token id
        uint256 baseTokenId = _tokenId % 100;

        // If there is no scheduled recipient:
        if (tokenRecipient[baseTokenId].wallet == address(0)) {
            return false;

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L263-L269

Impact

Founders lose their first NFT entitlement when reservedUntilTokenId is greater than 100.

Code Snippet

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L160-L176
https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L184-L194
https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/token/Token.sol#L263-L269

Tool used

Manual Review

Recommendation

To fix this issue, ensure the base token ID for a founder remains within the 0-100 range, regardless of reservedUntilTokenId:

        // Used to store the base token id the founder will recieve
-       uint256 baseTokenId = reservedUntilTokenId;
+       uint256 baseTokenId = reservedUntilTokenId > 100 ? reservedUntilTokenId % 100 : 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;
        }

Duplicate of #42