code-423n4 / 2022-09-nouns-builder-findings

10 stars 6 forks source link

Founders may not receive their vesting allocations #706

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In _addFounders the vesting allocations are distributed among the first one hundred nouns according to their ownership percentage. These first one hundred represent the token ID:s modulo 100 that should be minted to the nounders, which is how it is calculated in other functions (i.e. _tokenId % 100). However, this modulation is calculated in line 118 as (baseTokenId += schedule) % 100;, which does not apply the modulus to baseTokenId. This may result in a an allocation being set at a higher index than the 100th.

Proof of Concept

Suppose six founders each have 1%. They will be allocated indices 0, 1, 2, 3, 4, and 5. And suppose another founder has 20%. He will then recieve indices 5, 10, ..., 95, 100, 105 (a total of 20 indices). But indices 100 and 105 are useless as they are not remainders of 100, and he will only actually receive 18%.

Tools Used

Code inspection

Recommended Mitigation Steps

Change line 118 to baseTokenId = (baseTokenId + schedule) % 100;

GalloDaSballo commented 1 year ago

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