KupiaSec - The first founder will lose 1% ownership if reservedUntilTokenId >= 100.

[sherlock-admin2]

KupiaSec

high

The first founder will lose 1% ownership if reservedUntilTokenId >= 100.

Summary

The first founder will lose 1% ownership if reservedUntilTokenId >= 100.

Vulnerability Detail

While adding founders in _addFounders(), baseTokenId starts from reservedUntilTokenId.

File: nouns-protocol\src\token\Token.sol
161:                 uint256 baseTokenId = reservedUntilTokenId; //@audit doesn't work when >= 100
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:                 }

So if reservedUntilTokenId is equal to or greater than 100, the first founder will be a recipient of baseTokenId which is greater than 99.

But tokenRecipient is useful only for [0, 99] and it means the first founder loses 1% ownership.

Here is a coded POC which can be added to Token.t.sol.

    function testLoseOwnershipWithBigReserveId() public {
        uint256 reservedUntilTokenId = 100;
        deployAltMock(reservedUntilTokenId);

        uint256 expectedTotalOwnership = token.totalFounderOwnership();

        Founder memory thisFounder;

        uint actualTotalOwnership;
        for (uint256 i; i <= 99; ++i) {
            thisFounder = token.getScheduledRecipient(i);
            actualTotalOwnership += thisFounder.wallet != address(0) ? 1: 0;
        }

        assertEq(expectedTotalOwnership, actualTotalOwnership + 1);
    }

As we can see from the test, actualTotalOwnership is less than the expected value.

Impact

The first founder will lose 1% ownership if reservedUntilTokenId >= 100.

Code Snippet

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

Tool used

Manual Review

Recommendation

baseTokenId should be set like this.

    uint256 baseTokenId = reservedUntilTokenId % 100;

Duplicate of #42