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

Uneven Distribution

waydou

Summary

The MondrianWallet::tokenURI function in the contract returns different URIs based on the modulo of the token ID with a constant value. The implementation results in an uneven distribution, where one URI represents a significantly higher proportion of the results compared to the others.

Vulnerability Details

The function uses a modular arithmetic operation to determine which URI to return based on the token ID
* If the result is 0, the function returns ART_ONE.
* If the result is 1, the function returns ART_TWO.
* If the result is 2, the function returns ART_THREE.
* For all other results (3 through 9), the function returns ART_FOUR.
This indicates:
* ART_ONE has a 10% chance of being returned (1/10 possibilities).
* ART_TWO has a 10% chance (1/10 possibilities).
* ART_THREE has a 10% chance (1/10 possibilities).
* ART_FOUR has a 70% chance (7/10 possibilities).

Impact

The distribution is biased, with ART_FOUR having a much higher likelihood of being returned compared to the others.

Proof Of Code

You will need to add public mint function in MondrianWallet, I should note this mint function is only for proof purposes and should not be used as it's flawed

function mint(address to, uint256 _tokenIdCounter) public onlyOwner { _safeMint(to, _tokenIdCounter); // Use _safeMint for ERC-721 compliance }

Place the following into MondrianWallet.test.js.

it("should have an even distribution of token URIs", async function () {
const expectedURIs = [
"ar://jMRC4pksxwYIgi6vIBsMKXh3Sq0dfFFghSEqrchd_nQ",
"ar://8NI8_fZSi2JyiqSTkIBDVWRGmHCwqHT0qn4QwF9hnPU",
"ar://AVwp_mWsxZO7yZ6Sf3nrsoJhVnJppN02-cbXbFpdOME",
"ar://n17SzjtRkcbHWzcPnm0UU6w1Af5N1p0LAcRUMNP-LiM"
];
const walletOwner = await mondrianWallet.owner()
const distribution = [0, 0, 0, 0]; // To count occurrences of each URI
for (let i = 0; i < 100; i++) { // Mint 100 tokens
await mondrianWallet.mint(walletOwner,i); // You might need a mint function if it's not there
}
// Check token URIs for the first 100 tokens
for (let i = 0; i < 100; i++) {
const tokenURI = await mondrianWallet.tokenURI(i);
const index = expectedURIs.indexOf(tokenURI);
​
if (index >= 0) {
distribution[index]++; // Count which URI is returned
}
}
console.log(distribution)
// [ 10, 10, 10, 70 ]
});

Tools Used

Manual

Recommendations

with this each art has a 25% probability

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

Lead Judging Commences

inallhonesty Lead Judge about 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.