First Flight #15: Mondrian Wallet
First Flight #15
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

The art paintings are not equal distributed in `MondrianWallet::tokenURI`

bube

Summary

The ModrianWallet::tokenURI function returns one of 4 random Mondrian art paintings. Each painting should have equal distribution. But the implementation of the function gives 70% chance for one of the paintings.

Vulnerability Details

The ModrianWallet::tokenURI function distributes four art URIs. In the documentation is written:

You'll see the tokenURI function returns one of 4 random Mondrian art paintings. Each should have equal distribution and be random.

But the function doesn't distribute these 4 random Mondrian art painting equally:

​
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;
}
}
​

The distribution is based on modNumber. The modNumber is derived from tokenId % 10. Then if modNumber is 0, the user receives ART_ONE, if it is 1, the user receives ART_TWO, if it 2, the user receives ART_THREE and if it is another number the user receives ART_FOUR. That means the user has 10% chance to receive ART_ONE, ART_TWO, or ART_THREE and 70% chance to receive ART_FOUR.

Impact

To call MondrianWallet::tokenURI function the user should have tokenId. Therefore for the purpose of the test a function mintTo is created in the MondrianWallet. This function call the internal ERC721::_mint function:

function mintTo(address to, uint256 tokenId) external {
_mint(to, tokenId);
}

The following test case demonstrates the distribution of the 4 hardcoded URIs for 100 tokenIds. According to the documentation the different URIs should be equally distributed. But the chance the user to receive the one of the first three URIs is 10% and the chance for the fourth is 70%. That means the distribution is not equal. You can add the test to the file MondrianWallet.test.js.

​
it("distribution in tokenURI", async function () {
let artOne = 0
let artTwo = 0
let artThree = 0
let artFour = 0
let bob = "0x8626f6940E2eb28930eFb4CeF49B2d1F2C9C1199"
​
for (let i = 0; i < 100; ++i) {
await mondrianWallet.connect(await ethers.getSigner(bob)).mintTo(bob, i)
const uri = await mondrianWallet.connect(await ethers.getSigner(bob)).tokenURI(i)
if (uri == "ar://jMRC4pksxwYIgi6vIBsMKXh3Sq0dfFFghSEqrchd_nQ")
artOne = artOne + 1
else if (uri == "ar://8NI8_fZSi2JyiqSTkIBDVWRGmHCwqHT0qn4QwF9hnPU")
artTwo = artTwo + 1
else if (uri == "ar://AVwp_mWsxZO7yZ6Sf3nrsoJhVnJppN02-cbXbFpdOME")
artThree = artThree + 1
else if (uri == "ar://n17SzjtRkcbHWzcPnm0UU6w1Af5N1p0LAcRUMNP-LiM")
artFour = artFour + 1
else
return
}
console.log("artOne:", artOne)
console.log("artTwo:", artTwo)
console.log("artThree:", artThree)
console.log("artFour:", artFour)
});

The distribution of the different URIs for 100 tokenIds:

artOne: 10
artTwo: 10
artThree: 10
artFour: 70
​

Tools Used

Manual Review, Hardhat

Recommendations

You should change tokenId % 10 to tokenId % 4 in order to receive equal distribution for each art URI. It is worth noting that the use of tokenId is also incorrect, but that is a separated issue and it is described in another report.

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

If you execute the test after applying the proposed change to the code, you will see that the distribution of the arts is equal. The chance the user to receive one of the four arts URIs is 25%.

​
artOne: 25
artTwo: 25
artThree: 25
artFour: 25
Updates

Lead Judging Commences

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

NFT's should have equal distribution

Support

FAQs

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