Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Underpayment And Integer Division Rounding Vulnerability in buyOutEstateNFT

Summary

The InheritanceManager contract is designed to manage asset inheritance, enabling beneficiaries to buy out NFTs representing real-world assets by compensating other beneficiaries for their shares. However, a significant vulnerability exists in the buyOutEstateNFT function due to improper handling of integer division in Solidity. This flaw leads to:

  1. Underpayment to Beneficiaries: Beneficiaries receive less than their entitled share of the NFT’s value due to rounding errors caused by integer division truncation.

  2. Stuck Tokens: A portion of the tokens paid by the buyer remains locked in the contract, then later the remaining token in the contract will then be shared with both the buyer when the withdrawInheritedFunds function is called.

These issues stem from the order of operations in calculating the finalAmount and distributing shares, resulting in compounded precision loss. This flaw compromises the contract’s fairness, reliability, and financial integrity, potentially causing disputes and losses among the beneficiaries.

Vulnerability Details

The buyOutEstateNFT function allows a beneficiary to buy out an NFT by paying the other beneficiaries for their shares based on the NFT’s value. The relevant code is as follows:

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

Key Issues

  1. Integer Division in finalAmount Calculation

    • Formula: finalAmount = (value / divisor) * multiplier, where divisor = n (number of beneficiaries) and multiplier = n - 1.

    • Problem: Solidity’s integer division truncates remainders (e.g., 100 / 3 = 33). Performing division before multiplication discards the remainder early, resulting in an inaccurate finalAmount.

    • Intended Logic: The buyer should pay for the n - 1 other beneficiaries’ shares, each worth value / n. The correct total should be (value * (n - 1)) / n, but the current formula (value / n) * (n - 1) loses precision due to truncation.

  2. Compounded Rounding in Distribution

    • Formula: Each of the n - 1 other beneficiaries receives finalAmount / divisor (i.e., finalAmount / n).

    • Problem: Dividing finalAmount—already reduced by earlier truncation—further compounds the loss, underpaying beneficiaries.

    • Example:

      • For value = 100, n = 3:

        • finalAmount = (100 / 3) * 2 = 33 * 2 = 66.

        • Each beneficiary gets 66 / 3 = 22.

        • Total distributed: 22 * 2 = 44, leaving 66 - 44 = 22 tokens stuck.

      • Expected: Each share should be approximately 100 / 3 ≈ 33.333, with the buyer paying 66.666 and each beneficiary receiving 33.333.

  3. Stuck Tokens

    • The total amount distributed (44 in the example) is less than the finalAmount paid (66), leaving the difference (22) trapped in the contract.

Example Scenario

  • Parameters:

    • NFT Value (value): 100 units

    • Number of Beneficiaries (n): 3

    • Buyer: One beneficiary

  • Calculations:

    • divisor = 3

    • multiplier = 2

    • finalAmount = (100 / 3) * 2 = 33 * 2 = 66

    • Each of the two other beneficiaries receives 66 / 3 = 22.

    • Total distributed: 22 * 2 = 44.

    • Tokens stuck: 66 - 44 = 22.

  • Intended Outcome:

    • Each share: 100 / 3 ≈ 33.333.

    • Buyer pays: 33.333 * 2 ≈ 66.666.

    • Each beneficiary receives: 33.333.

  • Actual Outcome:

    • Beneficiaries receive 22 each (underpaid by ~11.333).

    • 22 units remain locked in the contract.

This discrepancy worsens with larger values or different beneficiary counts.

Impact

Underpayment to Beneficiaries

  • Beneficiaries receive significantly less than their fair share due to repeated truncation, violating the contract’s equitable distribution intent.

  • This could lead to disputes or financial harm to affected parties.

  • Stuck Tokens in the Contract

    • The difference between the buyer’s payment and the distributed amount remains inaccessible, effectively reducing the total value available to beneficiaries.

    • The token locked on the contract will later be shared with the buyer, potentially prompting legal or trust issues.

Tools Used

Manual Review

Recommendations

To address the flaw and ensure accurate, fair distribution, the following remediation steps are proposed:

  1. Adjust Calculation Order

    • Compute the per-beneficiary share first, then derive the total payment and distributions to reduce precision loss.

    • Updated Code:

uint256 share = value / divisor; // Individual share
uint256 totalPayment = share * multiplier; // Total buyer pays
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);
}
}
Updates

Lead Judging Commences

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

buyOutNFT has wrong denominator

truncation of integers

Support

FAQs

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