0xbepresent - The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens #67

sherlock-admin2 · 2023-12-01T16:27:14Z

0xbepresent

medium

The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens

Summary

The Token::setReservedUntilTokenId() function does not update the assigned founders in the tokenRecipient[] array causing founders being unable to claim some assigned tokens.

Vulnerability Detail

Founders are assigned in the tokenRecipient[] array once the token is initialized in the Token:_addFounders() function. The position assigned in the tokenRecipient[] array depends if the token is initialized using reservedUntilTokenId code line 161:

File: Token.sol
160:                 // Used to store the base token id the founder will recieve
161:                 uint256 baseTokenId = reservedUntilTokenId;
162: 
163:                 // For each token to vest:
164:                 for (uint256 j; j < founderPct; ++j) {
165:                     // Get the available token id
166:                     baseTokenId = _getNextTokenId(baseTokenId);
167: 
168:                     // Store the founder as the recipient
169:                     tokenRecipient[baseTokenId] = newFounder;
170: 
171:                     emit MintScheduled(baseTokenId, founderId, newFounder);
172: 
173:                     // Update the base token id
174:                     baseTokenId = (baseTokenId + schedule) % 100;
175:                 }

The issue arises when the reservedUntilTokenId can be updated using Token::setReservedUntilTokenId() function but the founders tokenRecipient[] will not be updated with the new reservedUntilTokenId value. As a result, some assigned tokens to the founder will be unclaimable.

Impact

Founders will not be able to claim some assigned tokens if the reservedUntilTokenId is updated because the assigned tokenRecipient is not updated. Please see the next scenario:

The token is initialized and one founder is assigned 99 ownershipPct and reservedUntilTokenId zero.
The founder is assigned the array tokenRecipient[] from 0 to 98.
The owner sets a new reservedUntilTokenId to 98 using the setReservedUntilTokenId() function.
After the auction begins, the founder will only receive one token, since during the minting process the tokenId will start at 98 ID and no previous token IDs will be minted for the founder.
The token ownership is transferred to the treasury causing that a proposal needs to be created if owner wants to update the founders list in order to fix the problem.

The next test reveals that the founder is assigned 99 initially, but ends up receiving only 1 token.

// File: Token.t.sol
// forge test --match-test "test_UpdateReserveUntilTokenIdWillNotUpdateTheAssignedFounders" -vvv
//
    function test_UpdateReserveUntilTokenIdWillNotUpdateTheAssignedFounders() public {
        // Founder will not able to get some tokens once the reservedUntilTokenId is updated
        //
        // 1. The token is initialized, one founder and 99 ownershipPct and reserverUntilTokenId zero
        createUsers(1, 1 ether);
        address[] memory wallets = new address[](1);
        uint256[] memory percents = new uint256[](1);
        uint256[] memory vestExpirys = new uint256[](1);
        address founder = otherUsers[0];
        wallets[0] = founder;
        percents[0] = 99;
        vestExpirys[0] = 4 weeks;
        deployWithCustomFounders(wallets, percents, vestExpirys);
        //
        // 2. The founder is assigned the array tokenRecipient[] from 0 to 98
        assertEq(token.totalFounders(), 1);
        assertEq(token.totalFounderOwnership(), 99);
        unchecked {
            for (uint256 i; i < 99; ++i) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        //
        // 3. Set a new reservedUntulTokenId = 98 and the founder still will have the same tokenRecipient[0]
        // causing the founder being unable to claim his tokens
        vm.prank(address(founder));
        token.setReservedUntilTokenId(98);
        unchecked {
            for (uint256 i; i < 99; ++i) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        //
        // 4. The Auction starts and founder obtain only one token
        vm.prank(address(auction));
        uint256 tokenId = token.mint();
        assertEq(token.balanceOf(founder), 1);
    }

Code Snippet

Token::setReservedUntilTokenId()

Tool used

Manual review

Recommendation

The founder tokenRecipient[] array should be updated using the new reservedUntilTokenId in the setReservedUntilTokenId() function:

    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();
        }
++      // Remove all previous assigned `tokenRecipient` from founders

        // Set the new reserve
        reservedUntilTokenId = newReservedUntilTokenId;

++      // Assign the new `tokenRecipient` founders array using the new reservedUntilTokenId

        emit ReservedUntilTokenIDUpdated(newReservedUntilTokenId);
    }

The text was updated successfully, but these errors were encountered:

neokry · 2023-12-06T19:03:17Z

This is not an issue. the founders allocation applies after the token reserve. in the given example a founder will still receive 99% of tokens after the reserve (ending with tokenId 98). they only receive one token in the example because they only checked until tokenId 98 which is the only non reserved token.

nevillehuang · 2023-12-08T17:03:01Z

Imo, this is also a duplicate of the family under #42 (see comments there), going to duplicate.

0xbepresent · 2023-12-14T17:08:05Z

Escalate

In order to explain why this issue is not a duplicated of #42 I need to provide some context.

The fix implemented here #42 (comment) DOES NOT fix the problem since if reservedUntilTokenId is a non-zero value, then some tokens assigned to the founders will not be able to be claimed, this is because the solution proposed here #42 DOES NOT take into account reservedUntilTokenId. For example:

The token is initialized with reservedUntilTokenId=90 and founder percent = 99, so with the fix the assignment would be:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 99|

As we see, the allocation for founders will only be 9 tokens since 89 tokens will be reserved for airdrops and marketing stuff. This can be seen in the following test which we see that the founder only ends up minting 9 tokens:

// File: Token.t.sol
//
    function test_FounderWontBeAbleToClaimTokensIfTheTokenIsInitializedUsingAReservedUntilTokenId() public {
        // Founders won't be able to claim assigned ownership tokens if the token is initialized using
        // non-zero reservedUntilTokenId
        //
        // 1. Token is deployed and a founder is assigned with owner ship of 99 and reservedUntilTokenId = 90.
        createUsers(1, 1 ether);
        address[] memory wallets = new address[](1);
        uint256[] memory percents = new uint256[](1);
        uint256[] memory vestExpirys = new uint256[](1);
        address founder = otherUsers[0];
        wallets[0] = founder;
        percents[0] = 99;
        vestExpirys[0] = 4 weeks;
        deployWithCustomFoundersAndCustomReserve(wallets, percents, vestExpirys, 90);
        //
        // 2. Assert the scheduledRecipient and the founder is assigned the position 0, to 99
        // That's is a problem because ids 0 to 90 are reserved and the founder won't be able to claim those tokens.
        assertEq(token.totalFounders(), 1);
        assertEq(token.totalFounderOwnership(), 99);
        unchecked {
            for (uint256 i = 0; i < 99; i++) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        //
        // 3. Auction starts and the founder is only able to claim 9 token because the tokenId starts at number 90
        // As a result, the previous assigned tokens to the founder will be unclaimable.
        vm.prank(address(auction));
        token.mint();
        assertEq(token.balanceOf(founder), 9);
    }

--- a/nouns-protocol/src/token/Token.sol
+++ b/nouns-protocol/src/token/Token.sol
@@ -158,7 +158,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                 uint256 schedule = 100 / founderPct;
 
                 // Used to store the base token id the founder will recieve
-                uint256 baseTokenId = reservedUntilTokenId;
+                uint256 baseTokenId = 0;
 
                 // For each token to vest:
                 for (uint256 j; j < founderPct; ++j) {

The best solution is the one proposed here #77 in which it involves reservedUntilTokenId, so in places where the modulo operator like % (100) is used, must be changed to % (100 + reservedUntilTokenId) so that the assignment is as follows:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 188|

Now the allocation for founders will be 99 tokens and the allocation for the reserve will be 89 tokens. As we see, the founders will NO longer lose tokens. This is demonstrated in the following test where the fix of changing % (100) to % (100 + reservedUntilTokenId) is implemented:

// File: Token.t.sol
//
    function test_ReservedUntilTokenIdFix() public {
        //
        // 1. Token is deployed and a founder is assigned with owner ship of 99 and reservedUntilTokenId = 90.
        createUsers(1, 1 ether);
        address[] memory wallets = new address[](1);
        uint256[] memory percents = new uint256[](1);
        uint256[] memory vestExpirys = new uint256[](1);
        address founder = otherUsers[0];
        wallets[0] = founder;
        percents[0] = 99;
        vestExpirys[0] = 4 weeks;
        deployWithCustomFoundersAndCustomReserve(wallets, percents, vestExpirys, 90);
        //
        // 2. Assert the scheduledRecipient and the founder is assigned the position 90,...,189 (90 + 99)
        assertEq(token.totalFounders(), 1);
        assertEq(token.totalFounderOwnership(), 99);
        unchecked {
            for (uint256 i = 90; i < 189; i++) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        // 3. Auction starts and the founder is able to claim all tokens
        vm.prank(address(auction));
        token.mint();
        assertEq(token.balanceOf(founder), 99);
    }

+++ b/nouns-protocol/src/token/Token.sol
@@ -171,7 +171,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                     emit MintScheduled(baseTokenId, founderId, newFounder);
 
                     // Update the base token id
-                    baseTokenId = (baseTokenId + schedule) % 100;
+                    baseTokenId = (baseTokenId + schedule) % (100 + reservedUntilTokenId);
                 }
             }
 
@@ -186,7 +186,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     function _getNextTokenId(uint256 _tokenId) internal view returns (uint256) {
         unchecked {
             while (tokenRecipient[_tokenId].wallet != address(0)) {
-                _tokenId = (++_tokenId) % 100;
+                _tokenId = (++_tokenId) % (100 + reservedUntilTokenId);
             }
 
             return _tokenId;
@@ -262,7 +262,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     /// @param _tokenId The ERC-721 token id
     function _isForFounder(uint256 _tokenId) private returns (bool) {
         // Get the base token id
-        uint256 baseTokenId = _tokenId % 100;
+        uint256 baseTokenId = _tokenId % (100 + reservedUntilTokenId);
 
         // If there is no scheduled recipient:
         if (tokenRecipient[baseTokenId].wallet == address(0)) {
@@ -367,7 +367,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     /// NOTE: If a founder is returned, there's no guarantee they'll receive the token as vesting expiration is not considered
     /// @param _tokenId The ERC-721 token id
     function getScheduledRecipient(uint256 _tokenId) external view returns (Founder memory) {
-        return tokenRecipient[_tokenId % 100];
+        return tokenRecipient[_tokenId % (100 + reservedUntilTokenId)];
     }
 
     /// @notice Update the list of allocation owners
@@ -414,7 +414,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                 for (uint256 j; j < cachedFounder.ownershipPct; ++j) {
                     // Get the next index that hasn't already been cleared
                     while (clearedTokenIds[baseTokenId] != false) {
-                        baseTokenId = (++baseTokenId) % 100;
+                        baseTokenId = (++baseTokenId) % (100 + reservedUntilTokenId);
                     }
 
                     delete tokenRecipient[baseTokenId];
@@ -423,7 +423,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                     emit MintUnscheduled(baseTokenId, i, cachedFounder);
 
                     // Update the base token id
-                    baseTokenId = (baseTokenId + schedule) % 100;
+                    baseTokenId = (baseTokenId + schedule) % (100 + reservedUntilTokenId);
                 }
             }
         }

Note: The following function has to be added in test/utils/NounsBuilderTest.sol for the tests to work:

+++ b/nouns-protocol/test/utils/NounsBuilderTest.sol
@@ -274,6 +274,25 @@ contract NounsBuilderTest is Test {
         setMockMetadata();
     }
 
+    function deployWithCustomFoundersAndCustomReserve(
+        address[] memory _wallets,
+        uint256[] memory _percents,
+        uint256[] memory _vestExpirys,
+        uint256 _reservedUntilTokenId
+    ) internal virtual {
+        setFounderParams(_wallets, _percents, _vestExpirys);
+
+        setMockTokenParamsWithReserve(_reservedUntilTokenId);
+
+        setMockAuctionParams();
+
+        setMockGovParams();
+
+        deploy(foundersArr, tokenParams, auctionParams, govParams);
+
+        setMockMetadata();
+    }
+

Now, given that context, the fix proposed here #77 DOES NOT solve the problem of this Issue, the problem of this issue #67 is that you have to delete all the founders' assignments and recalculate so that the founders are assigned correctly and do not lose tokens. Following the previous example and assuming that the fix proposed here #77 is applied:

The token is initialized with reservedUntilTokenId=90 and founder percent = 99, so with the fix the assignment would be:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 188|

Now, the owner updates reservedUntilTokenId using setReservedUntilTokenId() to reservedUntilTokenId=95, if the fix recommended here #67 is NOT implemented, then the founders will lose tokens since the allocation would be as follows:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 94|

And the following will be left for the founders
|95, 96, 97, ..., 188|

As you can see, the founder lost some tokens. So to fix this in setReservedUntilTokenId() you have to reassign the founders, that is, delete the previous assignments for the founder and assign the new assignment so that the distribution looks like this:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 94|

And the following will be left for the founders
|95, 96, 97, ..., 193|

As you can see, the founder will NO longer lose tokens.

In conclusion, this issue is different and is NOT a duplicate of #77 #42 since the solution proposed here must delete the previous assignment for the founders and assign again using a different reservedUntilTokenId and the solution proposed in #77 #42 does NOT solve the reallocation of tokens for founders.

Hope the information helps to solve the problem.

sherlock-admin2 · 2023-12-14T17:08:10Z

Escalate

In order to explain why this issue is not a duplicated of #42 I need to provide some context.

The fix implemented here #42 (comment) DOES NOT fix the problem since if reservedUntilTokenId is a non-zero value, then some tokens assigned to the founders will not be able to be claimed, this is because the solution proposed here #42 DOES NOT take into account reservedUntilTokenId. For example:

The token is initialized with reservedUntilTokenId=90 and founder percent = 99, so with the fix the assignment would be:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 99|

As we see, the allocation for founders will only be 9 tokens since 89 tokens will be reserved for airdrops and marketing stuff. This can be seen in the following test which we see that the founder only ends up minting 9 tokens:

// File: Token.t.sol
//
    function test_FounderWontBeAbleToClaimTokensIfTheTokenIsInitializedUsingAReservedUntilTokenId() public {
        // Founders won't be able to claim assigned ownership tokens if the token is initialized using
        // non-zero reservedUntilTokenId
        //
        // 1. Token is deployed and a founder is assigned with owner ship of 99 and reservedUntilTokenId = 90.
        createUsers(1, 1 ether);
        address[] memory wallets = new address[](1);
        uint256[] memory percents = new uint256[](1);
        uint256[] memory vestExpirys = new uint256[](1);
        address founder = otherUsers[0];
        wallets[0] = founder;
        percents[0] = 99;
        vestExpirys[0] = 4 weeks;
        deployWithCustomFoundersAndCustomReserve(wallets, percents, vestExpirys, 90);
        //
        // 2. Assert the scheduledRecipient and the founder is assigned the position 0, to 99
        // That's is a problem because ids 0 to 90 are reserved and the founder won't be able to claim those tokens.
        assertEq(token.totalFounders(), 1);
        assertEq(token.totalFounderOwnership(), 99);
        unchecked {
            for (uint256 i = 0; i < 99; i++) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        //
        // 3. Auction starts and the founder is only able to claim 9 token because the tokenId starts at number 90
        // As a result, the previous assigned tokens to the founder will be unclaimable.
        vm.prank(address(auction));
        token.mint();
        assertEq(token.balanceOf(founder), 9);
    }

--- a/nouns-protocol/src/token/Token.sol
+++ b/nouns-protocol/src/token/Token.sol
@@ -158,7 +158,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                 uint256 schedule = 100 / founderPct;
 
                 // Used to store the base token id the founder will recieve
-                uint256 baseTokenId = reservedUntilTokenId;
+                uint256 baseTokenId = 0;
 
                 // For each token to vest:
                 for (uint256 j; j < founderPct; ++j) {

The best solution is the one proposed here #77 in which it involves reservedUntilTokenId, so in places where the modulo operator like % (100) is used, must be changed to % (100 + reservedUntilTokenId) so that the assignment is as follows:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 188|

Now the allocation for founders will be 99 tokens and the allocation for the reserve will be 89 tokens. As we see, the founders will NO longer lose tokens. This is demonstrated in the following test where the fix of changing % (100) to % (100 + reservedUntilTokenId) is implemented:

// File: Token.t.sol
//
    function test_ReservedUntilTokenIdFix() public {
        //
        // 1. Token is deployed and a founder is assigned with owner ship of 99 and reservedUntilTokenId = 90.
        createUsers(1, 1 ether);
        address[] memory wallets = new address[](1);
        uint256[] memory percents = new uint256[](1);
        uint256[] memory vestExpirys = new uint256[](1);
        address founder = otherUsers[0];
        wallets[0] = founder;
        percents[0] = 99;
        vestExpirys[0] = 4 weeks;
        deployWithCustomFoundersAndCustomReserve(wallets, percents, vestExpirys, 90);
        //
        // 2. Assert the scheduledRecipient and the founder is assigned the position 90,...,189 (90 + 99)
        assertEq(token.totalFounders(), 1);
        assertEq(token.totalFounderOwnership(), 99);
        unchecked {
            for (uint256 i = 90; i < 189; i++) {
                assertEq(token.getScheduledRecipient(i).wallet, founder);
            }
        }
        // 3. Auction starts and the founder is able to claim all tokens
        vm.prank(address(auction));
        token.mint();
        assertEq(token.balanceOf(founder), 99);
    }

+++ b/nouns-protocol/src/token/Token.sol
@@ -171,7 +171,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                     emit MintScheduled(baseTokenId, founderId, newFounder);
 
                     // Update the base token id
-                    baseTokenId = (baseTokenId + schedule) % 100;
+                    baseTokenId = (baseTokenId + schedule) % (100 + reservedUntilTokenId);
                 }
             }
 
@@ -186,7 +186,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     function _getNextTokenId(uint256 _tokenId) internal view returns (uint256) {
         unchecked {
             while (tokenRecipient[_tokenId].wallet != address(0)) {
-                _tokenId = (++_tokenId) % 100;
+                _tokenId = (++_tokenId) % (100 + reservedUntilTokenId);
             }
 
             return _tokenId;
@@ -262,7 +262,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     /// @param _tokenId The ERC-721 token id
     function _isForFounder(uint256 _tokenId) private returns (bool) {
         // Get the base token id
-        uint256 baseTokenId = _tokenId % 100;
+        uint256 baseTokenId = _tokenId % (100 + reservedUntilTokenId);
 
         // If there is no scheduled recipient:
         if (tokenRecipient[baseTokenId].wallet == address(0)) {
@@ -367,7 +367,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
     /// NOTE: If a founder is returned, there's no guarantee they'll receive the token as vesting expiration is not considered
     /// @param _tokenId The ERC-721 token id
     function getScheduledRecipient(uint256 _tokenId) external view returns (Founder memory) {
-        return tokenRecipient[_tokenId % 100];
+        return tokenRecipient[_tokenId % (100 + reservedUntilTokenId)];
     }
 
     /// @notice Update the list of allocation owners
@@ -414,7 +414,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                 for (uint256 j; j < cachedFounder.ownershipPct; ++j) {
                     // Get the next index that hasn't already been cleared
                     while (clearedTokenIds[baseTokenId] != false) {
-                        baseTokenId = (++baseTokenId) % 100;
+                        baseTokenId = (++baseTokenId) % (100 + reservedUntilTokenId);
                     }
 
                     delete tokenRecipient[baseTokenId];
@@ -423,7 +423,7 @@ contract Token is IToken, VersionedContract, UUPS, Ownable, ReentrancyGuard, ERC
                     emit MintUnscheduled(baseTokenId, i, cachedFounder);
 
                     // Update the base token id
-                    baseTokenId = (baseTokenId + schedule) % 100;
+                    baseTokenId = (baseTokenId + schedule) % (100 + reservedUntilTokenId);
                 }
             }
         }

Note: The following function has to be added in test/utils/NounsBuilderTest.sol for the tests to work:

+++ b/nouns-protocol/test/utils/NounsBuilderTest.sol
@@ -274,6 +274,25 @@ contract NounsBuilderTest is Test {
         setMockMetadata();
     }
 
+    function deployWithCustomFoundersAndCustomReserve(
+        address[] memory _wallets,
+        uint256[] memory _percents,
+        uint256[] memory _vestExpirys,
+        uint256 _reservedUntilTokenId
+    ) internal virtual {
+        setFounderParams(_wallets, _percents, _vestExpirys);
+
+        setMockTokenParamsWithReserve(_reservedUntilTokenId);
+
+        setMockAuctionParams();
+
+        setMockGovParams();
+
+        deploy(foundersArr, tokenParams, auctionParams, govParams);
+
+        setMockMetadata();
+    }
+

Now, given that context, the fix proposed here #77 DOES NOT solve the problem of this Issue, the problem of this issue #67 is that you have to delete all the founders' assignments and recalculate so that the founders are assigned correctly and do not lose tokens. Following the previous example and assuming that the fix proposed here #77 is applied:

The token is initialized with reservedUntilTokenId=90 and founder percent = 99, so with the fix the assignment would be:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 89|

And the following will be left for the founders
|90, 91, 92, ..., 188|

Now, the owner updates reservedUntilTokenId using setReservedUntilTokenId() to reservedUntilTokenId=95, if the fix recommended here #67 is NOT implemented, then the founders will lose tokens since the allocation would be as follows:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 94|

And the following will be left for the founders
|95, 96, 97, ..., 188|

As you can see, the founder lost some tokens. So to fix this in setReservedUntilTokenId() you have to reassign the founders, that is, delete the previous assignments for the founder and assign the new assignment so that the distribution looks like this:

The following Token IDs assigned for the reservation (airdrops, marketings stuff)
|0, 1, ... , 94|

And the following will be left for the founders
|95, 96, 97, ..., 193|

As you can see, the founder will NO longer lose tokens.

In conclusion, this issue is different and is NOT a duplicate of #77 #42 since the solution proposed here must delete the previous assignment for the founders and assign again using a different reservedUntilTokenId and the solution proposed in #77 #42 does NOT solve the reallocation of tokens for founders.

Hope the information helps to solve the problem.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

nevillehuang · 2023-12-14T17:10:18Z

Will review all duplicates under #42, so please check deduplication comments here in the mean time.

neokry · 2023-12-14T17:16:47Z

founders are reserved a % of tokens not actual tokenIds. ie when you mention if reservedUntilTokenId is set to 90 founder will receive x% of all tokens minted after tokenId 90.

nevillehuang · 2023-12-15T05:51:03Z

Agree with sponsor, and as such, I believe #67, #89, #139 and #171 to be invalid.

Czar102 · 2023-12-20T11:41:41Z

I agree with invalidation, will allow for final comments, tagging @0xbepresent.

Czar102 · 2023-12-21T12:47:04Z

Result:
Invalid
Unique

sherlock-admin2 · 2023-12-21T12:47:11Z

Escalations have been resolved successfully!

Escalation status:

0xbepresent: accepted

github-actions bot added High A valid High severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Dec 6, 2023

neokry added the Sponsor Disputed The sponsor disputed this issue's validity label Dec 6, 2023

nevillehuang mentioned this issue Dec 8, 2023

bin2chen - when reservedUntilTokenId > 100 first funder loss 1% NFT #42

Open

nevillehuang added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Dec 8, 2023

nevillehuang closed this as completed Dec 8, 2023

sherlock-admin added the Reward A payout will be made for this issue label Dec 13, 2023

sherlock-admin added the Escalated This issue contains a pending escalation label Dec 14, 2023

Czar102 removed the High A valid High severity issue label Dec 21, 2023

sherlock-admin added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue labels Dec 21, 2023

sherlock-admin2 removed the Escalated This issue contains a pending escalation label Dec 21, 2023

sherlock-admin added the Escalation Resolved This issue's escalations have been approved/rejected label Dec 21, 2023

sherlock-admin removed the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Dec 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

0xbepresent - The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens #67

0xbepresent - The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens #67

sherlock-admin2 commented Dec 1, 2023 •

edited by Czar102

Loading

neokry commented Dec 6, 2023

nevillehuang commented Dec 8, 2023

0xbepresent commented Dec 14, 2023

sherlock-admin2 commented Dec 14, 2023

nevillehuang commented Dec 14, 2023

neokry commented Dec 14, 2023

nevillehuang commented Dec 15, 2023

Czar102 commented Dec 20, 2023

Czar102 commented Dec 21, 2023

sherlock-admin2 commented Dec 21, 2023

0xbepresent - The Token::setReservedUntilTokenId() function does not update the assigned founders in the tokenRecipient[] array causing founders being unable to claim some assigned tokens #67

0xbepresent - The Token::setReservedUntilTokenId() function does not update the assigned founders in the tokenRecipient[] array causing founders being unable to claim some assigned tokens #67

Comments

sherlock-admin2 commented Dec 1, 2023 • edited by Czar102 Loading

The Token::setReservedUntilTokenId() function does not update the assigned founders in the tokenRecipient[] array causing founders being unable to claim some assigned tokens

Summary

Vulnerability Detail

Impact

Code Snippet

Tool used

Recommendation

neokry commented Dec 6, 2023

nevillehuang commented Dec 8, 2023

0xbepresent commented Dec 14, 2023

sherlock-admin2 commented Dec 14, 2023

nevillehuang commented Dec 14, 2023

neokry commented Dec 14, 2023

nevillehuang commented Dec 15, 2023

Czar102 commented Dec 20, 2023

Czar102 commented Dec 21, 2023

sherlock-admin2 commented Dec 21, 2023

0xbepresent - The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens #67

0xbepresent - The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens #67

sherlock-admin2 commented Dec 1, 2023 •

edited by Czar102

Loading

The `Token::setReservedUntilTokenId()` function does not update the assigned founders in the `tokenRecipient[]` array causing founders being unable to claim some assigned tokens