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

Broken Beneficiary Removal Creating Array Gaps leading to Loss of funds

abhishekthakur

Summary

The removeBeneficiary function uses delete without restructuring the array, creating gaps that lead to funds sent to address(0) during distribution.

Vulnerability Details

Affected Code - https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/main/src/InheritanceManager.sol#L163C5-L166C6

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
///@custom: audit only value is reset to address(0), while index stays as is
@> delete beneficiaries[indexToRemove];
}

removeBeneficiaryis meant to remove a user that's either accidently added by the owner, or want to remove for any reasons so that funds could split b/w remaining beneficiaries only when time comes.


However, the current implementation only sets the element to the zero address, without actually removing the index. This create an issue during distribution, because when funds are distributed, these zero addresses will receive a share.

Ultimately those funds are lost forever. Some erc20 won't support moving funds to zero address, in that case funds will be locked forever.

POC

In existing inheritance test suite we add 2 more users

... imports as ...
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
+ address user2 = makeAddr("user2");
+ address user3 = makeAddr("user3");
.... other code

Then add the following test

function testBrokenBeneficiaryRemovalCostFundLoss() public {
// Add multiple beneficiaries
console.log("---- Initial State ----");
console.log(
"InheritanceManager balance:",
address(im).balance,
"ETH"
);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
​
// Remove the middle beneficiary
vm.prank(owner);
im.removeBeneficiary(user2);
​
// Fast forward time and initiate inheritance
vm.warp(block.timestamp + 91 days);
vm.prank(user1);
im.inherit();
​
// Withdraw funds
vm.prank(user1);
im.withdrawInheritedFunds(address(0));
​
// Check funds distribution
uint256 totalDistributed = user1.balance +
user2.balance +
user3.balance;
assert(totalDistributed < 5 ether); // Some ETH was sent to address(0)}
console.log("Total distributed:", totalDistributed, "ETH");
console.log(
"InheritanceManager balance after claim:",
address(im).balance,
"ETH"
);
console.log("total lost:", (5 ether - totalDistributed), "ETH");
​
}

after running forge test --mt testBrokenBeneficiaryRemovalCostFundLoss -vv we get following output

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] testBrokenBeneficiaryRemovalCostFundLoss() (gas: 261700)
Logs:
---- Initial State ----
InheritanceManager balance: 5000000000000000000 ETH
Total distributed: 3333333333333333332 ETH
InheritanceManager balance after claim: 2 ETH
total lost: 1666666666666666668 ETH
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.58ms (165.46µs CPU time)
​
Ran 1 test suite in 153.42ms (1.58ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

which confirms our finding.

Impact

Loss of funds

Tools Used

Foundry

Recommendations

Here is the recommendation to fix the issue -

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
// Move the last element to the position being removed
beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
// Remove the last element
beneficiaries.pop();
// Update the deadline
_setDeadline();
}
Updates

Lead Judging Commences

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

Incorrect removal from beneficiary list causes funds to be send to 0 address

Support

FAQs

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