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

The function `MondrianWallet:tokenURI` does not introduce any randomness itself; it is deterministic based on the value of tokenId.

vesper

Summary

The function MondrianWallet:tokenURI does not introduce any randomness, it is deterministic based on the value of tokenId.

Vulnerability Details

The randomness of the tokenURI function is directly tied to the randomness of the tokenId values. The output of this function is predictable and not random. This function does not introduce any randomness itself; it is deterministic based on the value of tokenId.

Impact

The randomness of the tokenURI function is not compliant with the NFT part annoucement:

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

Tools Used

Manual review

Recommendations

Refactor MondrianWallet.sol by integrating Chainlink VRF (Verifiable Random Function) for genuinely random URI selection:

import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
​
...
​
contract MondrianWallet is ERC721, Ownable, IAccount, VRFConsumerBase {
bytes32 internal keyHash;
uint256 internal fee;
mapping(uint256 => uint256) public tokenIdToRandomNumber;
mapping(bytes32 => uint256) private requestToTokenId;
​
string[4] private uris = [
"ar://jMRC4pksxwYIgi6vIBsMKXh3Sq0dfFFghSEqrchd_nQ",
"ar://8NI8_fZSi2JyiqSTkIBDVWRGmHCwqHT0qn4QwF9hnPU",
"ar://AVwp_mWsxZO7yZ6Sf3nrsoJhVnJppN02-cbXbFpdOME",
"ar://n17SzjtRkcbHWzcPnm0UU6w1Af5N1p0LAcRUMNP-LiM"
];
​
constructor(address entryPoint, address vrfCoordinator, address linkToken, bytes32 _keyHash, uint256 _fee)
ERC721("MondrianWallet", "MW")
VRFConsumerBase(vrfCoordinator, linkToken)
{
keyHash = _keyHash;
fee = _fee;
}
​
...
​
function requestNewRandomURI(uint256 tokenId) public returns (bytes32 requestId) {
require(LINK.balanceOf(address(this)) >= fee, "Not enough LINK");
requestId = requestRandomness(keyHash, fee);
requestToTokenId[requestId] = tokenId;
}
​
function fulfillRandomness(bytes32 requestId, uint256 randomness) internal override {
uint256 tokenId = requestToTokenId[requestId];
uint256 randomResult = randomness % 4;
tokenIdToRandomNumber[tokenId] = randomResult;
}
​
function tokenURI(uint256 tokenId) public view override returns (string memory) {
require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
uint256 modNumber = tokenIdToRandomNumber[tokenId];
return uris[modNumber];
}
​
...
​

  • VRFConsumerBase: The contract inherits from VRFConsumerBase, which is part of Chainlink's VRF functionality.

  • Variables for Chainlink VRF: keyHash, fee, and mappings to store randomness requests and results.

  • requestNewRandomURI(uint256 tokenId): A function that requests randomness. It requires some LINK token balance to pay for the request.

  • fulfillRandomness(bytes32 requestId, uint256 randomness): An internal function that Chainlink nodes call to deliver the randomness. It maps the random number to a tokenId.

Updated tokenURI(uint256 tokenId): Now it retrieves the URI based on the stored random result.

Updates

Lead Judging Commences

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

NFTs are not random

Support

FAQs

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