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

Incorrect payment calculation in InheritanceManager::buyOutEstateNFT()

sala1

Summary

Some beneficiaries do not receive their share while others do due to the logic.

Vulnerability Details

The code in consideration is:

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

The finalAmount = (value / divisor) * multiplier.

This amount is supposed to be split among the remaining beneficiaries.
However, inside the loop:
When msg.sender == beneficiaries[i], the function returns immediately, skipping the remaining payments.
This means some beneficiaries do not receive their share while others do.
But, the contract still burns the NFT, assuming the transaction was fair.

Consider this scenario : 4 Beneficiaries:

NFT value = 1000 USDC

4 beneficiaries: John, Pat, Charlie, and David

John wants to buy out the NFT

Calculation:

divisor = 4

multiplier = 4 - 1 = 3

finalAmount = (1000 / 4) * 3 = 750

John transfers 750 USDC to the contract.

Each beneficiary (except John) should get 750 / 3 = 250 USDC.

Incorrect Distribution:

Loop starts with i = 0:

If John is at i = 0, function returns immediately, and Pat, Charlie, and David get nothing.

If John is at i = 2, Pat and Charlie get 250 USDC each, but David gets nothing.

If John is not the first in the array, some beneficiaries are overpaid while others get nothing.

Impact

incorrect or un-evan payment to beneficiaries (bad specially in the case inheritance to living members)

Tools Used

Manual review

Recommendations

Modifications to be made:

  • The full finalAmount is distributed only among non-buying beneficiaries.

  • The return statement does not exit the loop prematurely.

if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / multiplier);
}
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 10 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.

Give us feedback!