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

Incorrect Payment Distribution in buyOutEstateNFT

lszb

Summary

The buyOutEstateNFT function incorrectly distributes payments when a beneficiary buys out an NFT. Instead of sending each of the other beneficiaries their full share of the NFT's value, it sends a reduced amount, leaving excess funds unallocated in the contract. This results in beneficiaries receiving less than their entitled share and the contract retaining funds that should have been distributed.

Vulnerability Details

The buyOutEstateNFT function calculates the total payment (finalAmount) as (value / N) * (N - 1), where N is the number of beneficiaries (divisor), representing the amount the buyer must pay to compensate the other N - 1 beneficiaries for their shares. However, when distributing this amount, the function sends finalAmount / N to each of the N - 1 other beneficiaries instead of the correct share, value / N. This leads to underpayment per beneficiary and a total distribution less than finalAmount, with the remainder left in the contract.

Vulnerable Code:

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

Impact

  • Underpayment to Beneficiaries: Each of the N - 1 other beneficiaries receives less than their fair share (value / N), undermining the intended payout mechanism.

  • Funds Left in Contract: The difference between finalAmount and the total distributed amount remains locked in the contract, inaccessible to beneficiaries.

  • Potential Disputes: The buyer pays the correct total amount, but the flawed distribution may lead to disagreements or loss of trust among beneficiaries.

Proof of Concept (PoC)

  1. Deploy the contract with 3 beneficiaries.

  2. Mint an NFT with value = 90 and assetToPay set to an ERC20 token.

  3. After inheritance is set (isInherited = true), one beneficiary calls buyOutEstateNFT.

  4. The function calculates finalAmount = (90 / 3) * 2 = 60.

  5. It transfers 60 units from the buyer to the contract.

  6. In the loop, it sends 60 / 3 = 20 to each of the 2 other beneficiaries, totaling 40.

  7. The remaining 20 units stay in the contract, and each beneficiary receives 20 instead of the expected 30.

Tools Used

  • Manual review

Recommendations

Modify the buyOutEstateNFT function to correctly calculate and distribute each beneficiary’s share:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 amountPerBeneficiary = value / divisor;
uint256 totalPayment = amountPerBeneficiary * (divisor - 1);
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), totalPayment);
for (uint256 i = 0; i < divisor; i++) {
if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], amountPerBeneficiary); // Correct calculation
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

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

buyOutNFT has wrong denominator

Support

FAQs

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

Give us feedback!