First Flight #15: Mondrian Wallet

Summary

The ModrianWallet::tokenURI function returns one of 4 random Mondrian art paintings. Each painting should be random distributed, but this is not implemented correctly.

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 in a random way these 4 Mondrian art paintings:

The function distributes the tokenURI based on modNumber. The modNumber is calculated by tokenId % 10. This means the distribution is based on the input argument tokenId. If the user calls twice the function, the user will receive the same tokenURI. If the distribution should be random, that means by every call to the function, the function should return different result (tokenURI).

Impact

The following test function shows that Bob calls twice the MondrianWallet::tokenURI function with the same tokenURI. In that case the function should return two different results, but due to the incorrect implementation, the function returns the same result in the both calls.

Tools Used

Manual Review, Hardhat

Recommendations

The most appropriate solution for random numbers is Chainlink VRF. But Chainlink does not provide its VRF v2 service on the zkSync rollup: https://docs.chain.link/vrf/v2/subscription/supported-networks.
Therefore, I suggest to generate random number off-chain and then use it in the MondrianWallet::tokenURI function instead of tokenId.