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

Incorrect Buyout Calculation in `InheritanceManager::buyOutEstateNFT` Results in Locked Funds and Unfair Distribution

summer

Summary

The buyOutEstateNFT function uses an incorrect mathematical formula to calculate buyout amounts, resulting in residual funds being permanently locked in the contract and beneficiaries receiving less than their fair share.

Finding Description

In the InheritanceManager contract, the buyOutEstateNFT function contains a critical mathematical error in how it calculates and distributes funds during an NFT buyout. The function is intended to allow one beneficiary to buy out others' shares of a jointly inherited NFT.
The problematic code is located in the buyOutEstateNFT function:

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);
}

The issue occurs in two parts:

  • Incorrect Calculation Order: The function calculates finalAmount as (value / divisor) * multiplier. In Solidity, which uses integer division, performing division before multiplication causes precision loss. This approach fundamentally breaks the financial invariant that each beneficiary should receive an equal portion of the NFT's value.

  • Double Division Problem: After the initial calculation of finalAmount, the function distributes it by again dividing by divisor in the line IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor). This second division results in significantly less tokens being distributed than were collected from the buyer.

The combination of these errors violates a core property of the contract: that beneficiaries should receive fair compensation for their share of the NFT. Furthermore, since the contract collects more tokens than it distributes, funds become permanently trapped in the contract with no mechanism to retrieve them.

Impact Explanation

it affects the buyOutEstateNFT function, which is used to distribute funds among beneficiaries. The incorrect calculation could result in beneficiaries receiving less than they should, potentially leading to disputes or financial loss.

Likelihood Explanation

very much likely to occur

Proof of Concept

Consider a scenario with 3 beneficiaries and an NFT valued at 100 USDC:

Inputs:
- value = 100 USDC
- beneficiaries.length = 3
- msg.sender is one of the beneficiaries
​
Calculations:
1. divisor = 3
2. multiplier = 2
3. finalAmount = (100 / 3) * 2 = 33 * 2 = 66 USDC
​
Execution:
- Contract collects 66 USDC from buyer
- Each recipient receives finalAmount / divisor = 66 / 3 = 22 USDC
- Total distributed: 22 USDC * 2 recipients = 44 USDC
- Remaining locked in contract: 66 - 44 = 22 USDC (33% of collected funds)

With more beneficiaries, the percentage of locked funds increases:

  • With 4 beneficiaries = 43% of funds become locked

  • With 10 beneficiaries = 73% of funds become locked

Recommendation

The function should be rewritten to:

  • Calculate each beneficiary's fair share directly

  • Transfer exactly that amount to each beneficiary (except the buyer)

  • Ensure the buyer pays the correct amount to the contract

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

Lead Judging Commences

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

buyOutNFT has wrong denominator

buyOutNFT has return instead of continue

Support

FAQs

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

Give us feedback!