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

Incorrect `buyOutEstateNFT` Distribution

kweks

The buyOutEstateNFT function has critical flaws in its payment distribution logic, leading to financial loss, unfair settlements, and protocol instability. Here’s a breakdown of the risks and their implications:

Code Snippet:

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


** Key Issues and Impacts**

a. Early Return Skips Beneficiaries

  • Problem: If the calling beneficiary is found in the loop, the function exits immediately with return, skipping all subsequent beneficiaries.

  • Example:

    • Beneficiaries: [Alice, Bob, Carol].

    • Alice calls buyOutEstateNFT.

    • The loop checks i=0 (Alice), triggers return, and exits.

    • Result: Bob and Carol receive nothing, even though Alice paid for the NFT.

b. Incorrect Payment Distribution

  • Formula Flaw: The finalAmount calculation (value / divisor) * multiplier is correct, but the distribution logic is broken:

    • finalAmount / divisor is sent to each beneficiary except the caller.

    • Example: value = 100, divisor = 3 → finalAmount = 66.

    • Each beneficiary should get 22 (66 / 3), but the loop skips payments due to the early return.

c. Funds Stuck in the Contract

  • Dust Accumulation: The truncated value / divisor leaves residual funds locked in the contract (e.g., 100 - 66 = 34 in the example above).

  • Risk: These funds are permanently lost, violating the protocol’s promise of fair distribution.

d. NFT Burned Prematurely

  • Problem: The NFT is burned (nft.burnEstate(_nftID)) even if payments to beneficiaries are incomplete.

  • Impact: The real-world asset’s on-chain representation is destroyed, but beneficiaries may not have received their fair share.


Attack Scenarios

Scenario 1: Malicious Beneficiary Exploit

  1. Setup:

    • Beneficiaries: [Alice, Bob, Carol].

    • NFT value: 100 USDC.

  2. Action: Alice calls buyOutEstateNFT.

  3. Result:

    • Alice pays 66 USDC (2/3 of 100).

    • Loop exits immediately (Alice is first in the array).

    • Bob and Carol receive 0 USDC.

    • NFT is burned.

  4. Profit for Alice: She effectively buys the NFT for 66 USDC instead of the intended 66 USDC split between Bob and Carol.

Scenario 2: Dust Accumulation

  1. Setup:

    • Beneficiaries: [Alice, Bob].

    • NFT value: 101 USDC.

  2. Action: Alice calls buyOutEstateNFT.

  3. Result:

    • finalAmount = (101 / 2) * 1 = 50 USDC.

    • Alice pays 50 USDC.

    • Loop skips Bob (due to return), so Bob gets 0 USDC.

    • Residual 51 USDC (101 - 50) is locked forever.


Impact:

a. Financial Loss for Beneficiaries

  • Legitimate beneficiaries receive nothing or less than owed, violating the protocol’s core promise of fair distribution.

b. Protocol Instability

  • Trust in the system erodes if users observe funds being burned or unevenly distributed.

c. Inconsistent State

  • Burning the NFT before completing payouts creates mismatches between on-chain and real-world ownership.


Fixing the Issue

Step 1: Remove the Early return

Ensure the loop processes all beneficiaries:

for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender != beneficiaries[i]) { // Skip the caller only
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}

Step 2: Handle Residual Dust

Add the remainder to the last beneficiary’s share:

uint256 remainder = finalAmount % divisor;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender != beneficiaries[i]) {
uint256 share = (finalAmount / divisor);
if (i == beneficiaries.length - 1) {
share += remainder; // Distribute remainder
}
IERC20(assetToPay).safeTransfer(beneficiaries[i], share);
}
}

Step 3: Burn NFT After Successful Payouts

Ensure the NFT is burned only after all transfers succeed:

nft.burnEstate(_nftID); // Call after the loop

By fixing the loop logic and residual handling, the protocol ensures fair payouts and consistent state management.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 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!