founderPct variable cast

[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#L102 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L108

Vulnerability details

Description

There is a function _addFounders in Token contract. It accepts array of FounderParams as an input. For each of founders it uses founderPct as an variable to store percent ownership for such founder.

It is unsafe to cast it to from uint256 to uint8 in some of places, but not all:

...
if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();
...
newFounder.ownershipPct = uint8(founderPct);
...
uint256 schedule = 100 / founderPct;
...
// For each token to vest:
for (uint256 j; j < founderPct; ++j) {
    ...
}
...

Impact

Lack of casting founderPct to uint8 type can lead to incorrect calculation of schedule variable and incorrect number of iterations in the loop that goes for each token to vest for such founder.

Recommended Mitigation Steps

Use uint8 type for ownershipPct in FounderParams struct or cast founderPct to uint8 type in all possible places in _addFounders function.

[GalloDaSballo]

Dup of #303