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

Missing NFT distribution for Mondrian Wallets

paprikrumplikas

Summary

No NFT is distributed to the created Mondrian Wallets.

Vulnerability Details

As per the README, MondrianWallet instances should receive a Mondrian NFT. In practice, however, they do not receive one: MondrianWallet does not mint or distribute any NFTs.

The following test (written in Foundry) demonstrates that a user creates a Mondrian Wallet, but does not receive any NFTs.
For any tokenId, MondrianWallet::tokenURI will revert with custom error MondrainWallet__InvalidTokenId().

Proof of Code
function testNoNftIsMinted() public {
// user sets up a Mondrian Wallet
address owner = makeAddr("owner");
vm.prank(owner);
MondrianWallet mondrianWallet;
mondrianWallet = new MondrianWallet(address(entryPoint));
​
bytes memory expectedRevertReason = abi.encodeWithSignature("ERC721NonexistentToken(uint256)", 0);
​
vm.expectRevert(expectedRevertReason);
string memory art = mondrianWallet.tokenURI(0);
//console.log("Art: ", art);
​
uint256 balanceMW = mondrianWallet.balanceOf(address(mondrianWallet));
uint256 balanceOwner = mondrianWallet.balanceOf(owner);
​
console.log("Mondrian wallet balance: ", balanceMW);
console.log("Owner balance: ", balanceOwner);
}

Impact

Mondrian Wallets do not receive the Mondrian NFTs that they are entitled to.

Tools Used

Manual review, Foundry.

Recommendations

Modify MondrianWallet so that an NFT is minted when a MondrianWallet instance is created.

Regarding the code below, do note that it contains a suggestion only for fixing the absence of NFT minting and distribution. However, the distribution will not be fully as desired. For the desired outcome you have to make a design choice:

  • either ensure that tokenId is a random uint256 (by using ChainLink VRFv2 for deployment on Ethereum, and some other oracle for ZkSync), or

  • decouple the account abstraction and NFT functionality by not inheriting ĘRC721inMondrianWallet`, but rather creating a separate MondrianNFT contract, and then handle all NFT-related logic therein.

...
​
/*//////////////////////////////////////////////////////////////
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
IEntryPoint private immutable i_entryPoint;
+ uint256 tokenId = 1; // @note the value of tokenId should be random
​
/*//////////////////////////////////////////////////////////////
MODIFIERS
//////////////////////////////////////////////////////////////*/
modifier requireFromEntryPoint() {
if (msg.sender != address(i_entryPoint)) {
revert MondrianWallet__NotFromEntryPoint();
}
_;
}
​
modifier requireFromEntryPointOrOwner() {
if (msg.sender != address(i_entryPoint) && msg.sender != owner()) {
revert MondrianWallet__NotFromEntryPointOrOwner();
}
_;
}
​
/*//////////////////////////////////////////////////////////////
FUNCTIONS
//////////////////////////////////////////////////////////////*/
constructor(address entryPoint) Ownable(msg.sender) ERC721("MondrianWallet", "MW") {
i_entryPoint = IEntryPoint(entryPoint);
+ _mint(address(this), tokenId); // Mint to the new Mondrian Wallet
​
}
​
...
Updates

Lead Judging Commences

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

The Wallet doesn't end up owning any nft

Support

FAQs

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