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

Incorrect Payment Distribution in buyOutEstateNFT

osuolale

Summary

The buyOutEstateNFT function contains a flawed calculation for distributing payments among beneficiaries, leading to unequal and incorrect payouts. The formula (value / divisor) * multiplier (where divisor is the number of beneficiaries and multiplier is beneficiaries.length - 1) does not correctly split the NFT’s value, causing some beneficiaries to receive less than their fair share and leaving funds stuck in the contract due to rounding and logic errors.

Vulnerability Details

The vulnerable code is in the buyOutEstateNFT function:

solidity

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
//@audit-issue k wrong calculation
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);
}

Issues:

  1. Incorrect Payment Calculation:

    • finalAmount = (value / divisor) * multiplier computes the total amount the caller pays, where value is the NFT’s worth (e.g., 60e6 USDC), divisor is the number of beneficiaries (e.g., 3), and multiplier is beneficiaries.length - 1 (e.g., 2).

    • Example: For value = 60e6, divisor = 3, multiplier = 2:

      • finalAmount = (60e6 / 3) * 2 = 20e6 * 2 = 40e6.

    • This suggests the caller pays 40e6 USDC, but the intent seems to be that each of the other beneficiaries (2 in this case) receives a fair share of the NFT’s value.

  2. Uneven Distribution:

    • The payout to each non-caller beneficiary is finalAmount / divisor:

      • 40e6 / 3 = 13,333,333 USDC (with integer division).

    • With 3 beneficiaries, only 2 receive payouts (due to the return for the caller), so total distributed = 13,333,333 * 2 = 26,666,666.

    • The caller pays 40e6, but only 26,666,666 is distributed, leaving 40e6 - 26,666,666 = 13,333,334 USDC stuck in the contract.

  3. Logic Flaw:

    • The NFT’s value (60e6) should ideally be split equally among the non-caller beneficiaries (e.g., 30e6 each for 2 others with 3 total), but the calculation underestimates this.

    • The early return prevents proper iteration, and the burn happens even if distribution is incomplete.

POC

// SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
​
​
/// @title General InheritanceManager Tests
/// @notice Runs additional functional tests on `InheritanceManager`
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
​
// ERC20Mock requires a name, symbol, and initial supply
usdc = new ERC20Mock();
weth = new ERC20Mock();
}
​
function test_buyOutEstateNFTWrongCalculaion() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.warp(1);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.createEstateNFT("our beach-house", 60e6, address(usdc));
vm.stopPrank();
​
usdc.mint(user3, 60e6);
console.log("balance of user3: ", usdc.balanceOf(address(user3)));
console.log("balance of user2: ", usdc.balanceOf(address(user2)));
console.log("balance of user1: ", usdc.balanceOf(address(user1)));
console.log("balance of user: ", usdc.balanceOf(address(im)));
vm.warp(1 + 90 days);
vm.startPrank(user3);
usdc.approve(address(im), 60e6);
im.inherit();
im.buyOutEstateNFT(1);
vm.stopPrank();
​
console.log("balance of user3: ", usdc.balanceOf(address(user3)));
console.log("balance of user2: ", usdc.balanceOf(address(user2)));
console.log("balance of user1: ", usdc.balanceOf(address(user1)));
console.log("balance of user: ", usdc.balanceOf(address(im)));
​
assertNotEq(usdc.balanceOf(address(user2)), 20e6);
​
}
​
}

Test Evidence/Result:

From the test test_buyOutEstateNFTWrongCalculaion:

  • Initial State: 3 beneficiaries (user1, user2, user3), NFT value = 60e6 USDC.

  • Post-Execution (user3 calls):

    • user3 balance: 20e6 (60e6 - 40e6 paid).

    • user2 balance: 13,333,333.

    • user1 balance: 13,333,333.

    • contract balance: 13,333,334.

  • Expected: Each non-caller (user1, user2) should receive 30e6 (half of 60e6), totaling 60e6 paid by user3, with 0 remaining in the contract.

  • Actual: Incorrect payouts and 13,333,334 USDC stuck.

Impact

Funds Stuck: Approximately 1/3 of the payment (13,333,334 USDC in the test) remains in the contract, inaccessible without additional withdrawal mechanisms.

  • Unequal Distribution: Beneficiaries receive less than their fair share (13,333,333 vs. 30e6 expected), violating the intent of equitable buyout.

  • Financial Loss: The caller overpays relative to what’s distributed, and stuck funds reduce the contract’s usability.

  • Trust Issue: Incorrect payouts undermine confidence in the inheritance system.

Tools Used

Manual Review and Foundry

Recommendations

Correct the calculation to ensure the caller pays the full NFT value, equally distributed to other beneficiaries:

solidity

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length - 1; // Number of non-caller beneficiaries
uint256 amountPerBeneficiary = value / divisor;
uint256 totalAmount = value; // Caller pays full value
​
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), totalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
nft.burnEstate(_nftID);
}
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.