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

Early exit from NFT payment distribution loop leads to beneficiaries not receiving their expected share of the funds

iii

Summary

When a beneficiary buys an estate NFT through function InheritanceManager::buyOutEstateNFT, it's supposed to distribute the funds from the purchase evenly to the remaining beneficiaries. However, unless the beneficiary buying is the last one in the beneficiaries array, some of the beneficiaries will not receive their fair share of the payment.

Vulnerability Details

After calculating the amount to transfer from the buyer, the function loops through the beneficiaries array to distribute to each their shares of the payment. In order to not send funds to the purchasing beneficiary, the loop exits when encountering the sender, thus skipping the remaining beneficiaries and leaving the funds in the contract. Additionally the NFT is never burned.

Because the owner is most likely not available, the only way for them to receive their funds is by calling InheritanceManager::withdrawInheritedFunds which will evenly distribute the funds to all beneficiaries. Assuming they all act in good faith (as they are part of the same inheritance agreement), they'll need to coordinate with each other so those who did receive their share of the payment, send their withdrawn amount to those who didn't. If the beneficiary list is long enough that might be very difficult.

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;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}

Impact

Beneficiaries do not receive their expected share of the NFT payment funds.

Tools Used

Manual review

Recommendations

Replace the early function exit to ensure all beneficiaries (except the sender) receives their share:

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;
- } else {
+ 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.