baseTokenId variable calculation

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L71 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L118

Vulnerability details

Description

There is _addFounders function in the Token contract. There is the following loop:

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

// 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 += schedule) % 100;
}

As we can see line (baseTokenId += schedule) % 100; have a bug, there is no reason to do % 100 operation over the result value of the incrementing baseTokenId by schedule.

Recommended Mitigation Steps

We recommend removal of such modulo operation. If case it is necessary to perform a modulo operation over baseTokenId inside the loop, it will be correct to do the following:

baseTokenId += schedule;
baseTokenId %= 100;

[GalloDaSballo]

Dup of https://github.com/code-423n4/2022-09-nouns-builder-findings/issues/499