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

Double Division in Estate NFT Buyout Causes Beneficiary Underpayment

invcbull

Summary

The buyOutEstateNFT function incorrectly distributes payments by applying division twice, resulting in beneficiaries receiving only half their entitled share during NFT buyouts.

Vulnerability Details

Affected code:

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L258C4-L278C1


For a 1000 USDC NFT with 2 beneficiaries:

// First division in finalAmount calculation
finalAmount = (1000 / 2) * 1 = 500 USDC // Correct payment amount
​
// Second incorrect division during distribution
distribution = finalAmount / 2 = 250 USDC // Wrong! Should be 500 USDC

The distribution calculation divides the payment amount twice by the number of beneficiaries.

Initial State:

  • Two beneficiaries registered

  • NFT value: 1000 USDC

  • Inheritance activated

Step 1: User2 initiates buyout

  • Should pay: 500 USDC (1000/2 * 1)

  • Actually pays: 500 USDC (correct)

Step 2: Distribution occurs

  • User1 should receive: 500 USDC

  • Actually receives: 250 USDC (500/2) due to second division

POC :

function test_buyoutDistributionBug() public {
// Setup two beneficiaries
address user2 = makeAddr("user2");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.createEstateNFT("valuable estate", 1000e6, address(usdc));
vm.stopPrank();
​
// Enable inheritance
vm.warp(block.timestamp + 91 days);
vm.prank(user1);
im.inherit();
​
// Record initial balances
uint256 contractBalanceBefore = usdc.balanceOf(address(im));
uint256 user1BalanceBefore = usdc.balanceOf(user1);
uint256 user2BalanceBefore = usdc.balanceOf(user2);
​
// Execute buyout from user2
usdc.mint(user2, 1000e6);
vm.startPrank(user2);
usdc.approve(address(im), 1000e6);
im.buyOutEstateNFT(1);
​
// Verify that:
// 1. user2 pays 500e6 (half of 1000e6)
// 2. user1 should receive 250e6 (half of what user2 paid)
// 3. 250e6 remains in contract
assertEq(
usdc.balanceOf(user2),
user2BalanceBefore + 1000e6 - 500e6,
"User2 paid incorrect amount"
);
assertEq(
usdc.balanceOf(user1) - user1BalanceBefore,
250e6, // Should receive half of payment
"User1 received incorrect distribution"
);
assertEq(
usdc.balanceOf(address(im)) - contractBalanceBefore,
250e6, // Half of payment
"Incorrect amount in contract"
);
​
vm.stopPrank();
}

POC Result :

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_buyoutDistributionBug() (gas: 420975)
Traces:
[420975] InheritanceManagerTest::test_buyoutDistributionBug()
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]
├─ [0] VM::label(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], "user2")
│ └─ ← [Return]
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [69020] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Stop]
├─ [145826] InheritanceManager::createEstateNFT("valuable estate", 1000000000 [1e9], ERC20Mock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f])
│ ├─ [95512] NFTFactory::createEstate("valuable estate")
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], tokenId: 1)
│ │ ├─ emit MetadataUpdate(_tokenId: 1)
│ │ └─ ← [Return] 1
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(7862401 [7.862e6])
│ └─ ← [Return]
├─ [0] VM::prank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [22686] InheritanceManager::inherit()
│ └─ ← [Stop]
├─ [2559] ERC20Mock::balanceOf(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA]) [staticcall]
│ └─ ← [Return] 0
├─ [2559] ERC20Mock::balanceOf(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF]) [staticcall]
│ └─ ← [Return] 0
├─ [2559] ERC20Mock::balanceOf(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]) [staticcall]
│ └─ ← [Return] 0
├─ [44784] ERC20Mock::mint(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], 1000000000 [1e9])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], value: 1000000000 [1e9])
│ └─ ← [Stop]
├─ [0] VM::startPrank(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Return]
├─ [24735] ERC20Mock::approve(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 1000000000 [1e9])
│ ├─ emit Approval(owner: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], spender: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 1000000000 [1e9])
│ └─ ← [Return] true
├─ [51920] InheritanceManager::buyOutEstateNFT(1)
│ ├─ [24058] ERC20Mock::transferFrom(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 500000000 [5e8])
│ │ ├─ emit Transfer(from: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 500000000 [5e8])
│ │ └─ ← [Return] true
│ ├─ [23204] ERC20Mock::transfer(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], 250000000 [2.5e8])
│ │ ├─ emit Transfer(from: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], to: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], value: 250000000 [2.5e8])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [559] ERC20Mock::balanceOf(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]) [staticcall]
│ └─ ← [Return] 500000000 [5e8]
├─ [0] VM::assertEq(500000000 [5e8], 500000000 [5e8], "User2 paid incorrect amount") [staticcall]
│ └─ ← [Return]
├─ [559] ERC20Mock::balanceOf(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF]) [staticcall]
│ └─ ← [Return] 250000000 [2.5e8]
├─ [0] VM::assertEq(250000000 [2.5e8], 250000000 [2.5e8], "User1 received incorrect distribution") [staticcall]
│ └─ ← [Return]
├─ [559] ERC20Mock::balanceOf(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA]) [staticcall]
│ └─ ← [Return] 250000000 [2.5e8]
├─ [0] VM::assertEq(250000000 [2.5e8], 250000000 [2.5e8], "Incorrect amount in contract") [staticcall]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.02ms (1.85ms CPU time)

Impact

  • In this case beneficiaries receive only half of their entitled share during NFT buyouts causing severe underpayment.

  • Violates core business logic of fair asset distribution (breaks invariant 4 in project document)

  • Economic loss for beneficiaries in estate settlements

Tools Used

Manual Review and Foundry

Recommendations

Fix double division issue in distribution by calculating correct share per beneficiary based on remaining beneficiaries count (in this case : sharePerBeneficiary)

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);
+ uint256 sharePerBeneficiary = finalAmount / (beneficiaries.length - 1);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
continue;
- return;
} else {
- IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+ IERC20(assetToPay).safeTransfer(beneficiaries[i], sharePerBeneficiary);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

buyOutNFT has wrong denominator

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.