Submission Details
Severity:
Precision Loss and Incorrect Fund Distribution in `InheritanceManager::buyOutEstateNFT` and `InheritanceManager::withdrawInheritedFundsLeaves` Stale Funds in Contract
Summary
Precision Loss and Incorrect Fund Distribution
Vulnerability Details
Both InheritanceManager::buyOutEstateNFT and InheritanceManager::withdrawInheritedFundsLeaves functions use integer division without remainder handling, leading to permanently locked ETH/tokens in the contract. This affects all asset distributions, violating the protocol’s intent to fully disburse funds to beneficiaries.
Impact
High Severity
Stale Funds: Residual tokens accumulate in the contract, permanently locked.
Beneficiary Losses: Users receive fewer tokens than entitled, proportional to the truncation error.
Tools Used
Manual code review
Foundry test case (provided)
PoC
In withdrawInheritedFunds:
function test_withdrawFunds() public {
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 20);
console.log("Money on the contract before the division to the Beneficieries:", address(im).balance);
vm.warp(1 + 91 days);
vm.startPrank(user1);
im.inherit();
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
console.log("User1 have after the division:", user1.balance);
console.log("User2 have after the division:", user2.balance);
console.log("User3 have after the division:", user3.balance);
console.log("Money on the contract after the division:", address(im).balance);
}
[PASS] test_withdrawFundsDivision() (gas: 276902)
Logs:
Money on the contract before the division to the Beneficieries: 20
User1 have after the division: 6
User2 have after the division: 6
User3 have after the division: 6
Money on the contract after the division: 2
In buyOutEstateNFT
function test_removeOneBeneficieryLockNft() public {
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.createEstateNFT("our beach-house", 22, address(usdc));
vm.stopPrank();
vm.startPrank(user3);
usdc.mint(user3, 14);
usdc.approve(address(im), 22);
vm.warp(1 + 90 days);
im.inherit();
im.buyOutEstateNFT(1);
console.log("User1 Balance:", usdc.balanceOf(user1));
console.log("User2 Balance:", usdc.balanceOf(user2));
console.log("User3 Balance:", usdc.balanceOf(user3));
console.log("Money on the contract after the division:", usdc.balanceOf(address(im)));
}
Ran 1 test for test/Mytest.t.sol:Testcontract
[PASS] test_NFTFunds() (gas: 436381)
Logs:
User1 Balance: 4
User2 Balance: 4
User3 Balance: 0
Money on the contract after the division: 10000000000000000006
Recommendations
Restructure payments to collect exactly value tokens from the caller and distribute all tokens proportionally:
For withdrawInheritedFunds:
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
+ uint256 remainder = ethAmountAvailable % divisor;
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
- (bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
+ uint256 amountToSend = amountPerBeneficiary + (i == 0 ? remainder : 0);
+ (bool success,) = beneficiary.call{value: amountToSend}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
+ uint256 remainder = assetAmountAvailable % divisor;
for (uint256 i = 0; i < divisor; i++) {
- IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
+ uint256 amountToSend = amountPerBeneficiary + (i == 0 ? remainder : 0);
+ IERC20(_asset).safeTransfer(beneficiaries[i], amountToSend);
}
}
}
For buyOutEstateNFT:
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 paymentPerBeneficiary = value / divisor;
+ uint256 remainder = value % divisor;
+ IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), value);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
- IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+ uint256 amountToSend = paymentPerBeneficiary + (i == 0 ? remainder : 0);
+ IERC20(assetToPay).safeTransfer(beneficiaries[i], amountToSend);
}
}
nft.burnEstate(_nftID);
}
Updates
Lead Judging Commences
0xtimefliez 9 months ago
Submission Judgement Published Assigned finding tags:
truncation of integers
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.