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 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!