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