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

InheritanceManager Contract - `assetToPay` Overwrite Vulnerability in `createEstateNFT()` function

roccomania

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/main/src/InheritanceManager.sol#L143-L147

Summary

The createEstateNFT function in the InheritanceManager contract has a critical vulnerability where the assetToPay state variable is overwritten every time a new Estate NFT is created. This means only the most recently specified asset address will be considered valid for all Estate NFTs, rendering the intended per-NFT asset payment scheme ineffective and leading to potential loss of funds or unintended asset transfers.

Vulnerability Details

1. createEstateNFT Function

  • Description: This function allows the owner to create an NFT representing an underlying asset (like real estate). When an NFT is created, the owner specifies a _description, _value, and _asset.

  • Issue: The assetToPay state variable is overwritten with each new NFT creation, as shown in this line:

function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
@> assetToPay = _asset;
}

  • The assetToPay state variable is declared as a single address.

  • This variable is updated with _asset each time createEstateNFT is called.

  • There is no mapping or array to store multiple assetToPay addresses corresponding to different NFTs.

2. buyOutEstateNFT Function

  • Description: This function uses assetToPay to determine the payment asset for buying out an Estate NFT.

  • Issue: The function relies on the assetToPay state variable, which only reflects the most recently set asset:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
//.. (Rest of the function)
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
//.. (Rest of the function)
}

  • This means that only the last assetToPay will be used, regardless of the NFT being bought out.

Impact

1. Incorrect Payment Asset

  • When beneficiaries attempt to buy out an Estate NFT using the buyOutEstateNFT function, they will always be required to pay in the _asset that was specified for the last NFT created.

2. Loss of Funds

  • If different NFTs are intended to be paid with different assets, beneficiaries could send funds to the wrong asset if they're not aware of this flaw. The owner could also get confused, causing a significant loss of funds.

3. Unintended NFT Buy

  • Beneficiaries may buy an NFT with a different asset than intended.

Tools Used

  • Manual Code Review

Recommendations

1. Use a Mapping for assetToPay

  • Change assetToPay from a single address to a mapping that associates NFT IDs with payment asset addresses.

- address assetToPay;
+ mapping(uint256 => address) public assetToPay;

2. Update createEstateNFT

  • Modify createEstateNFT to store the _asset in the mapping, using the nftID as the key.

function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
- assetToPay = _asset;
+ assetToPay[nftID] = _asset;
}

3. Update buyOutEstateNFT

  • Modify buyOutEstateNFT to retrieve the correct asset address from the mapping.

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier; // @audit-issue Division before multiplication
- IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
+ IERC20(assetToPay[_nftID]).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return; // @audit-issue This will stop the function even after sending to some beneficiaries and won't burn NFT
} else {
- IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+ IERC20(assetToPay[_nftID]).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!