Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Randomness in `tokenURI` is flawed and does not follow the documentation

Description

tokenURI returns the URI of a token. However, this function contains several problems:

  • It returns an URI for an arbitrary tokenId and not the one owned by the wallet.

  • Randomness is just tokenId % 10 which allows every user to create several wallets or wait to have the image they want (by watching the mempool, it's easily possible to anticipate the next tokenId).

  • Documentation specifies that every painting has the same probability to be distributed. The code uses a modulo 10 and ART_FOUR is distributed every time the result is not 0, 1, or 2. It means there is a 7/10 chance to have ART_FOUR and 1/10 to have the others. That's not equal probability.

function tokenURI(
@> uint256 tokenId
) public view override returns (string memory) {
if (ownerOf(tokenId) == address(0)) {
revert MondrainWallet__InvalidTokenId();
}
@> uint256 modNumber = tokenId % 10;
if (modNumber == 0) {
return ART_ONE;
} else if (modNumber == 1) {
return ART_TWO;
} else if (modNumber == 2) {
return ART_THREE;
@> } else {
return ART_FOUR;
}
}

Risk

Likelyhood: High

  • Every wallet creation.

Impact:

  • Probabilities are not fair.

  • Randomness is easily predictable.

Recommended Mitigation

  • Use an oracle to generate a random number.

  • Use a modulo 4 to have an equal repartition of paintings.

  • Return the URI of the token owned by the wallet and make the NFT soulbound.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

NFT's should have equal distribution

NFTs are not random

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.