Description
The withdrawInheritedFunds() function in the InheritanceManager contract contains a critical vulnerability related to integer division rounding. The function calculates the amount each beneficiary should receive by dividing the total available amount by the number of beneficiaries, which can lead to funds being permanently trapped in the contract.
function withdrawInheritedFunds(address _asset) external {
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;
}
}
Since Solidity performs integer division with truncation (rounding down), any remainder after division will be lost and remain in the contract forever if the total amount is not evenly divisible by the number of beneficiaries.
Impact
This vulnerability has significant financial implications:
-
Permanently Locked Funds: Remainders from division operations remain locked in the contract with no mechanism to retrieve them.
-
Incomplete Inheritance: Beneficiaries don't receive their complete share of the inheritance, contradicting the contract's purpose.
-
Asset Depreciation: For certain less frequently used ERC20 tokens, these small locked amounts could become effectively worthless if the token contract is upgraded or deprecated.
-
Growing Impact: The issue becomes more severe as the number of beneficiaries increases, because the potential remainder becomes larger relative to the total distribution.
Proof of Concept
The following test case demonstrates the vulnerability:
function test_withdrawInheritedFundsRoundingIssue() public {
vm.warp(1);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(makeAddr("user2"));
im.addBeneficiery(makeAddr("user3"));
vm.deal(address(im), 10 ether);
vm.stopPrank();
vm.warp(1 + 90 days);
vm.prank(user1);
im.inherit();
uint256 initialContractBalance = address(im).balance;
console.log("Initial contract balance:", initialContractBalance);
vm.prank(user1);
im.withdrawInheritedFunds(address(0));
uint256 remainingBalance = address(im).balance;
assertGt(remainingBalance, 0, "Remaining balance should be greater than 0");
}
Output:
│ └─ ← [Stop]
├─ [0] console::log("Initial contract balance:", 10000000000000000000 [1e19]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::prank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [105010] InheritanceManager::withdrawInheritedFunds(0x0000000000000000000000000000000000000000)
│ ├─ [0] user1::fallback{value: 3333333333333333333}()
│ │ └─ ← [Stop]
│ ├─ [0] user2::fallback{value: 3333333333333333333}()
│ │ └─ ← [Stop]
│ ├─ [0] user3::fallback{value: 3333333333333333333}()
│ │ └─ ← [Stop]
│ └─ ← [Stop]
├─ [0] VM::assertGt(1, 0, "Remaining balance should be greater than 0") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Recommendation
-
Use safeMath library to handle arithmetic operation
-
Implement a more accurate distribution mechanism that handles the remainder:
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 beneficiaryCount = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 baseAmount = ethAmountAvailable / beneficiaryCount;
uint256 remainder = ethAmountAvailable % beneficiaryCount;
for (uint256 i = 0; i < beneficiaryCount; i++) {
address payable beneficiary = payable(beneficiaries[i]);
uint256 payment = baseAmount;
if (remainder > 0) {
payment += 1;
remainder--;
}
(bool success,) = beneficiary.call{value: payment}("");
require(success, "ETH transfer failed");
}
} else {
uint256 assetAmount = IERC20(_asset).balanceOf(address(this));
uint256 baseAmount = assetAmount / beneficiaryCount;
uint256 remainder = assetAmount % beneficiaryCount;
for (uint256 i = 0; i < beneficiaryCount; i++) {
uint256 payment = baseAmount;
if (remainder > 0) {
payment += 1;
remainder--;
}
IERC20(_asset).safeTransfer(beneficiaries[i], payment);
}
}
}
Tools Used
