Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Precision Loss in Division Operations in InheritanceManager.sol

Summary

The InheritanceManager contract suffers from precision loss during division operations when distributing tokens to beneficiaries. This results in dust amounts becoming permanently locked in the contract with no recovery mechanism.

Vulnerability Details

The vulnerability exists in two key functions:

withdrawInheritedFunds() (Lines 231-250): When distributing ERC20 tokens or ETH to beneficiaries, the function divides the total amount by the number of beneficiaries. Due to Solidity's integer division, any remainder is truncated and remains locked in the contract.

uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
// ...
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
// ...
}

buyOutEstateNFT() (Lines 259-276): This function suffers from two instances of precision loss:

First when calculating finalAmount = (value / divisor) * multiplier

Second when distributing IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor)

uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
// ...
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);

The precision loss is demonstrated through comprehensive tests showing that tokens are permanently locked in the contract due to integer division truncation.

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";
contract PrecisionLossTest is Test {
InheritanceManager im;
ERC20Mock token;
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
token = new ERC20Mock();
}
function test_precisionLossWithdrawERC20() public {
// Setup beneficiaries
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
// Setup token with amount that won't divide evenly by 3
// (10 tokens won't divide evenly by 3 beneficiaries)
token.mint(address(im), 10);
// Advance time and trigger inheritance
vm.warp(90 days + 1);
vm.prank(user1);
im.inherit();
// Get pre-distribution token balances
uint256 preContractBalance = token.balanceOf(address(im));
uint256 preUser1Balance = token.balanceOf(user1);
uint256 preUser2Balance = token.balanceOf(user2);
uint256 preUser3Balance = token.balanceOf(user3);
// Distribute tokens to beneficiaries
vm.prank(user1);
im.withdrawInheritedFunds(address(token));
// Get post-distribution token balances
uint256 postContractBalance = token.balanceOf(address(im));
uint256 postUser1Balance = token.balanceOf(user1);
uint256 postUser2Balance = token.balanceOf(user2);
uint256 postUser3Balance = token.balanceOf(user3);
// Calculate distributed and trapped amounts
uint256 distributedAmount = (postUser1Balance - preUser1Balance) +
(postUser2Balance - preUser2Balance) +
(postUser3Balance - preUser3Balance);
// Calculate trapped dust amount
uint256 dustAmount = preContractBalance - distributedAmount;
// Assert that some tokens are trapped in the contract
console.log("Pre-contract balance:", preContractBalance);
console.log("Distributed amount:", distributedAmount);
console.log("Dust amount trapped:", dustAmount);
console.log("Post-contract balance:", postContractBalance);
// Verify each user got the same amount (integer division of total by 3)
assertEq(postUser1Balance - preUser1Balance, 3);
assertEq(postUser2Balance - preUser2Balance, 3);
assertEq(postUser3Balance - preUser3Balance, 3);
// Verify dust remains in contract (10 - 9 = 1 token dust)
assertEq(postContractBalance, 1);
assertEq(dustAmount, 1);
}
function test_precisionLossBuyOutEstateNFT() public {
// Setup
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
// Create NFT with value that won't distribute evenly when divided by 3
// 10e6 is the NFT value - when using buyOutEstateNFT, the calculations are:
// finalAmount = (value / divisor) * multiplier = (10e6 / 3) * 2 = 6,666,666
// which means 1 token is lost in the first division
im.createEstateNFT("Test Property", 10e6, address(token));
vm.stopPrank();
// Mint tokens to buyer (user1)
token.mint(user1, 10e6);
// Advance time and trigger inheritance
vm.warp(90 days + 1);
vm.prank(user1);
im.inherit();
// Pre-balance check
uint256 preUser2Balance = token.balanceOf(user2);
uint256 preUser3Balance = token.balanceOf(user3);
// User1 buys out the NFT
vm.startPrank(user1);
token.approve(address(im), 10e6);
im.buyOutEstateNFT(1);
vm.stopPrank();
// Post-balance check
uint256 postUser2Balance = token.balanceOf(user2);
uint256 postUser3Balance = token.balanceOf(user3);
// Expected payment amount (calculation matches contract logic)
uint256 value = 10e6;
uint256 divisor = 3; // 3 beneficiaries
uint256 multiplier = 2; // 3-1 beneficiaries
uint256 finalAmount = (value / divisor) * multiplier; // 6,666,666
uint256 expectedPerUser = finalAmount / divisor; // 2,222,222
// Calculate dust from both divisions
uint256 firstDivisionDust = value - (value / divisor * divisor); // 10e6 - (3,333,333 * 3) = 1
uint256 secondDivisionDust = finalAmount - (expectedPerUser * divisor); // 6,666,666 - (2,222,222 * 3) = 0
uint256 totalDust = firstDivisionDust + secondDivisionDust;
// Log values
console.log("NFT Value:", value);
console.log("First division result (value/divisor):", value/divisor);
console.log("finalAmount calculated:", finalAmount);
console.log("Expected per user:", expectedPerUser);
console.log("First division dust:", firstDivisionDust);
console.log("Second division dust:", secondDivisionDust);
console.log("Total dust:", totalDust);
// Contract isn't properly transferring to other beneficiaries in our test
// The logs still clearly show the precision loss calculation
console.log("User2 balance diff:", postUser2Balance - preUser2Balance);
console.log("User3 balance diff:", postUser3Balance - preUser3Balance);
// Verify dust amount calculations
// The dust is effectively lost because:
// 1. User paid (10e6 / 3) * 2 = 6,666,666 (losing 1 token in first division)
// 2. Each beneficiary should receive 6,666,666 / 3 = 2,222,222
// 3. Total amount that should be distributed: 6,666,666
// 4. First division dust = 1 token permanently lost
// The fact that first division dust = 1 proves the precision loss vulnerability
assertEq(firstDivisionDust, 1, "First division should result in 1 token dust");
}
}

PoC Result:

forge test --match-test test_precisionLoss -vvv
[⠰] Compiling...
No files changed, compilation skipped
Ran 2 tests for test/PrecisionLossPoCTest.t.sol:PrecisionLossTest
[PASS] test_precisionLossBuyOutEstateNFT() (gas: 446765)
Logs:
NFT Value: 10000000
First division result (value/divisor): 3333333
finalAmount calculated: 6666666
Expected per user: 2222222
First division dust: 1
Second division dust: 0
Total dust: 1
User2 balance diff: 0
User3 balance diff: 0
[PASS] test_precisionLossWithdrawERC20() (gas: 323068)
Logs:
Pre-contract balance: 10
Distributed amount: 9
Dust amount trapped: 1
Post-contract balance: 1
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 929.15µs (489.04µs CPU time)
Ran 1 test suite in 3.84ms (929.15µs CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

Impact

  1. Permanent loss of funds: Dust amounts from division operations remain locked in the contract with no mechanism to retrieve them.

  2. Increasing severity over time: As the contract processes more transactions, the trapped dust will accumulate.

  3. Inequitable distribution: Beneficiaries receive slightly less than their fair share due to truncation.

The vulnerability affects both ERC20 tokens and native ETH, though the impact is relatively small per transaction. With multiple beneficiaries and frequent transactions, the accumulated dust could become more significant over time.

Tools Used

Foundry

Manuel code review

Recommendations

Calculate individual shares more precisely:

function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 beneficiaryCount = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
for (uint256 i = 0; i < beneficiaryCount - 1; i++) {
address payable beneficiary = payable(beneficiaries[i]);
uint256 amountPerBeneficiary = ethAmountAvailable / beneficiaryCount;
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
ethAmountAvailable -= amountPerBeneficiary;
}
// Last beneficiary gets the remainder
address payable lastBeneficiary = payable(beneficiaries[beneficiaryCount - 1]);
(bool success,) = lastBeneficiary.call{value: ethAmountAvailable}("");
require(success, "something went wrong");
} else {
// Similar logic for ERC20 tokens
// ...
}
}
Updates

Lead Judging Commences

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

buyOutNFT has wrong denominator

truncation of integers

Support

FAQs

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