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

Improper Beneficiary Removal Leaves Zero Address in Array, Enabling Irreversible Fund Loss

sulfurpt

Summary

Improper Beneficiary Removal Leaves Zero Address in Array, Enabling Irreversible Fund Loss

Vulnerability Details

The InheritanceManager::removeBeneficiary function does not correctly manage the beneficiaries array, leaving a zero address in the array after removal. This causes subsequent fund distributions (e.g., in InheritanceManager::buyOutEstateNFT and InheritanceManager::withdrawInheritedFunds) to send tokens to the zero address, permanently burning funds.

Impact

High Severity

Permanent Fund Loss: Tokens sent to the zero address are irretrievable.

Incorrect Distribution Logic: The presence of a zero address skews payment calculations (e.g., beneficiaries.length includes invalid entries).

Tools Used

Manual code review
Foundry test case (provided)

PoC

function test_withdrawFundsRemoveBeneficiary() 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.removeBeneficiary(user2);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 15);
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("Contract 0 have after the division:", address(0x0).balance);
}
Ran 1 test for test/Mytest.t.sol:Testcontract
[PASS] test_withdrawFundsRemoveBeneficiary() (gas: 261117)
Logs:
Money on the contract before the division to the Beneficieries: 15
User1 have after the division: 5
User2 have after the division: 0
User3 have after the division: 5
Contract 0 have after the division: 5

Recommendations

Replace the beneficiary to be removed with the last element in the array and then call .pop() to delete the last entry.

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
- delete beneficiaries[indexToRemove];
+ beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
+ beneficiaries.pop();
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 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.

Give us feedback!