Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Global state override for `assetToPay` ends with mishandling of funds when creating multiple NFTs

phylax

Summary

In InheritanceManager::createEstateNFT function the _assetparameter is set to the global variable assetToPay. This becomes an issue when multiple estate NFTs are priced in various assets because the asset to pay is set to what the last createEstateNFTfunction call sets it to.

Vulnerability Details

This test will revert because the last created NFT sets the assetToPayto usdc. When user1does not have usdcbut only wethin wallet the transaction reverts:

function testCreateMultipleEstateNFTs() public {
address user2 = makeAddr("user2");
​
vm.startPrank(owner);
im.createEstateNFT("our beach-house", 2000000, address(weth));
im.createEstateNFT("car", 2000000, address(usdc));
im.addBeneficiery(user1);
im.addBeneficiery(user2);
vm.stopPrank();
​
weth.mint(user1, 2000000);
vm.warp(1 + 90 days);
​
vm.startPrank(user1);
weth.approve(address(im), 2000000);
im.inherit();
vm.expectRevert();
im.buyOutEstateNFT(1);
vm.stopPrank();
}

Impact

This could lead to:

  1. Beneficiaries bying NFTs with wrong assets.

  2. Previous NFT payment requirements being silently changed.

  3. Potential loss of funds if assets have different values.

Tools Used

Manual code review, Claude 3.7 Sonnet

Recommendations

In the InheritanceManagercontract add this code:

// Add a mapping to track which NFT should be payed in which asset
+ mapping(uint256 nftID => address assetToPay) nftAsset;
​
​
function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
- assetToPay = _asset;
+ nftAsset[nftID] = _asset; // Store asset per NFT instead
}
​
function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
+ address asset = nftAsset[_nftID];
- IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
+ IERC20(asset).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
- IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+ IERC20(asset).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

global asset in NFT values

Support

FAQs

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

Give us feedback!