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

Incorrect Buyout Logic in buyOutEstateNFT

dumbbutt0

Summary

The buyOutEstateNFT function has two major flaws: (1) it stops paying beneficiaries after encountering the caller, leaving later beneficiaries unpaid, and (2) it miscalculates payouts, leaving funds stuck in the contract. This allows a beneficiary to buy out an NFT while underpaying others and trapping assets.

Vulnerability Details

How the Exploit Works

When a beneficiary buys out an NFT, the function calculates a total payout (finalAmount) but distributes it unevenly due to flawed logic. It stops processing beneficiaries after the caller and uses an incorrect per-beneficiary amount, leaving some unpaid and funds unspent.

Step-by-Step Exploit Execution

  1. Setup:

    • The contract has three beneficiaries: beneficiary1, beneficiary2, beneficiary3.

    • An NFT is created with a value of 300 ether (in an ERC20 token).

    • After 90 days, beneficiary1 inherits the contract.

  2. Exploit:

    • beneficiary2 calls buyOutEstateNFT(1) to buy the NFT.

    • The function calculates:

      • finalAmount = (300 / 3) * 2 = 200 ether (for two other beneficiaries).

      • Per-beneficiary payout = 200 / 3 ≈ 66.67 ether.

    • It pays beneficiary1 (~66.67 ether) but stops at beneficiary2 (the caller), leaving beneficiary3 unpaid.

  3. Result:

    • beneficiary1 gets ~66.67 ether.

    • beneficiary3 gets 0 ether.

    • beneficiary2 pays 200 ether, but only ~66.67 ether is distributed, leaving ~133.33 ether stuck in the contract.

Impact

Impact: Beneficiaries are underpaid or unpaid, and funds remain locked in the contract.

  • Root Cause:

    • The loop exits early when msg.sender is found.

    • The payout calculation divides the total amount incorrectly among beneficiaries.

PoC

function test_BuyOutIncompletePayment() public {
token = new ERC20Mock();
vm.startPrank(owner);
manager = new InheritanceManager();
nft = new NFTFactory(address(manager));
manager.addBeneficiery(beneficiary1);
manager.addBeneficiery(beneficiary2);
manager.addBeneficiery(beneficiary3);
manager.createEstateNFT("House", 300 ether, address(token));
vm.stopPrank();
​
token.mint(beneficiary2, 300 ether);
vm.prank(beneficiary2);
token.approve(address(manager), 300 ether);
​
vm.warp(block.timestamp + 90 days + 1);
vm.prank(beneficiary1);
manager.inherit();
​
uint256 initialBalance1 = token.balanceOf(beneficiary1);
uint256 initialBalance3 = token.balanceOf(beneficiary3);
​
vm.prank(beneficiary2);
manager.buyOutEstateNFT(1);
​
uint256 finalAmount = (300 ether / 3) * 2; // 200 ether
uint256 expectedPayout = finalAmount / 3; // ~66.67 ether
assertEq(token.balanceOf(beneficiary1) - initialBalance1, expectedPayout); // Paid
assertEq(token.balanceOf(beneficiary3) - initialBalance3, 0); // Not paid
}

Tools Used

Slither ,ai , personal scanner, foundry

Recommendations

Issue

The buyOutEstateNFT function has two problems:

  1. It stops paying beneficiaries after encountering the caller, leaving remaining beneficiaries unpaid.

  2. It miscalculates payouts, underpaying beneficiaries and trapping funds in the contract.

Fix

Revise buyOutEstateNFT to pay all non-caller beneficiaries correctly and calculate payouts accurately:

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

Explanation

  • Change 1: Replaced early loop termination with a condition to skip the caller, ensuring all other beneficiaries are paid.

  • Change 2: Fixed payout calculation to value / divisor per beneficiary, with totalPayout as (divisor - 1) * amountPerBeneficiary.

  • Why It Works: The loop now processes all beneficiaries, and the corrected math ensures funds are fully distributed without leftovers.

  • Best Practice: In payout logic, process all recipients and verify calculations to prevent fund misdistribution or trapping.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 5 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.