Submission Details
Severity:
[L-2] Integer division precision loss in `InheritanceManager::withdrawInheritedFunds` leads to unused wei and tokens stranded on `InheritanceManager` contract
Description: In InheritanceManager::withdrawInheritedFunds, calculating the amount per beneficiary for both ether and ERC20 tokens does not account for the remainder in integer division.
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}Integer division precision loss in `InheritanceManager::withdrawInheritedFunds` leads to unused wei and tokens stranded on `InheritanceManager` contract
uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor; // @audit integer division remainder is lost
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor; // @audit integer division remainder is lost
for (uint256 i = 0; i < divisor; i++) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}
Impact: A very small amount of wei and ERC20 tokens can be left in the contract InheritanceManager.
Proof of Concept:
Add the following unit test to InheritanceManagerTest.t.sol:
function test_precisionLossOnWithdrawal() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
// Add user1, user2, and user3 as beneficiaries
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
// Deposit 200 ether into InheritanceManager contract
vm.warp(1);
vm.deal(address(im), 200e18);
// Mint 200 USDC tokens for InheritanceManager contract
usdc.mint(address(im), 200e6);
// Skip ahead 90 days so timelock is expired
vm.warp(1 + 90 days);
// Inherit the funds
vm.startPrank(user1);
im.inherit();
// Withdraw ether from InheritanceManager contract
im.withdrawInheritedFunds(address(0));
// Withdraw USDC tokens from InheritanceManager contract
im.withdrawInheritedFunds(address(usdc));
vm.stopPrank();
console.log("user1.balance", user1.balance);
console.log("user2.balance", user2.balance);
console.log("user3.balance", user3.balance);
// Ether is still left in the InheritanceManager contract
console.log("Inheritance Manager contract balance = ", address(im).balance);
assertNotEq(address(im).balance, 0);
console.log("usdc.balanceOf(user1)", usdc.balanceOf(user1));
console.log("usdc.balanceOf(user2)", usdc.balanceOf(user2));
console.log("usdc.balanceOf(user3)", usdc.balanceOf(user3));
// USDC still left with InheritanceManager contract
console.log("usdc.balanceOf(address(im)) = ", usdc.balanceOf(address(im)));
assertNotEq(usdc.balanceOf(address(im)), 0);
}
Run the unit test with verbosity: forge test --mt test_precisionLossOnWithdrawal -vvv:
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_precisionLossOnWithdrawal() (gas: 429261)
Logs:
amountRemaining 2
user1.balance 66666666666666666666
user2.balance 66666666666666666666
user3.balance 66666666666666666666
Inheritance Manager contract balance = 2
usdc.balanceOf(user1) 66666666
usdc.balanceOf(user2) 66666666
usdc.balanceOf(user3) 66666666
usdc.balanceOf(address(im)) = 2
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.67ms (496.21µs CPU time)
Recommended Mitigation: Calculate the remainder and disperse to beneficiaries round-robin, knowing not everyone will get the small leftover.
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 ethLeftOver = ethAmountAvailable % divisor; // Calculate remainder
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
+ uint256 amountToSend = amountPerBeneficiary;
+ // Distribute leftover 1 wei at a time in round-robin fashion
+ if (i < ethLeftOver) {
+ amountToSend += 1;
+ }
- (bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
+ (bool success,) = beneficiary.call{value: amountToSend}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
+ uint256 tokenLeftOver = assetAmountAvailable % divisor; // Calculate remainder
for (uint256 i = 0; i < divisor; i++) {
+ uint256 amountToSend = amountPerBeneficiary;
+ // Distribute leftover tokens 1 at a time in round-robin fashion
+ if (i < tokenLeftOver) {
+ amountToSend += 1;
+ }
- IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
+ IERC20(_asset).safeTransfer(beneficiaries[i], amountToSend);
}
}
}
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.