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

Premature return in InheritanceManager::buyOutEstateNFT prevents proper fund distribution among beneficiaries

freesultan

Summary

The buyOutEstateNFT function, restricted to beneficiaries via the onlyBeneficiaryWithIsInherited modifier, contains a for loop that distributes payment to all beneficiaries except the caller (msg.sender). However, the loop uses a return statement when it encounters the caller’s address, exiting the function entirely and preventing subsequent beneficiaries from receiving their share. This disrupts the intended equal distribution of funds and leaves the NFT unburned in some cases, undermining the function’s purpose.

Vulnerability Details

In the following part of the code, the buyOutEstateNFTintends to distribute payment to all beneficiaries except the caller (msg.sender) . It calculates the value of the NFT (nftValue[_nftID]) and splits it among beneficiaries. Transfers finalAmount (total value minus the caller’s share) from msg.sender to the contract. Loops through beneficiaries to distribute finalAmount / divisor to each beneficiary except the caller. Burns the NFT (nft.burnEstate(_nftID)) after distribution.

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L271

The issue is that in the for loop when msg.sender matches a beneficiary (which it must, due to the modifier), the return statement exits the function immediately.

Consequence:

  • Beneficiaries after the caller’s index in the array receive nothing.

  • nft.burnEstate(_nftID) is never executed, leaving the NFT intact.

Impact

Beneficiaries listed after the caller in the array receive no payment, violating the intended equal split among all non-calling beneficiaries.

The contract retains finalAmount without distributing it fully, potentially locking funds if no other mechanism withdraws them.

The NFT remains unburned.

Tools Used

Manual code review

Recommendations

Replace return with continue to skip the caller and proceed with distribution.

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
- return;
+ continue;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
​

or remove the else clause:

for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
​
Updates

Lead Judging Commences

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

buyOutNFT has return instead of continue

Support

FAQs

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