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

Zero address handling vulnerabilities in beneficiary management can lead to permanent fund loss

swarecito

Summary

The InheritanceManager contract lacks proper zero address validation when managing beneficiaries. It allows zero addresses to be added as beneficiaries and uses delete on array elements which converts entries to the zero address without removing them. When inheritance is triggered, the contract attempts to distribute funds to all addresses in the array, including zero addresses, which can result in funds being permanently lost.

Vulnerability Details

There are three key components to this vulnerability:

  1. The addBeneficiery function doesn't validate against zero addresses:

function addBeneficiery(address _beneficiary) external onlyOwner {
// No zero address check
beneficiaries.push(_beneficiary);
_setDeadline();
}

  1. The removeBeneficiary function uses delete which sets the array element to address(0) but keeps it in the array:

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove]; // Sets to address(0) but doesn't remove the element
}

  1. The withdrawInheritedFunds function attempts to send funds to all addresses in the array without checking for zero addresses:

function withdrawInheritedFunds(address _asset) external {
// ...
uint256 divisor = beneficiaries.length;
// ...
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
// ...
}

When inheritance occurs, the contract will try to distribute funds equally to all entries in the beneficiaries array. If the array contains zero addresses, either from direct addition or as a result of using delete, the contract will attempt to send funds to address(0). This either results in the funds being permanently lost or in a transaction failure that prevents any beneficiary from receiving their inheritance.

Impact

This vulnerability can lead to several serious consequences:

  1. Permanent loss of funds: If ETH is sent to address(0), it becomes permanently inaccessible

  2. Inheritance distribution failure: Attempting to send tokens to address(0) may cause the entire distribution to fail

  3. Incorrect distribution calculations: Zero addresses in the array are counted in the divisor, leading to each beneficiary receiving less than their fair share

  4. Confusion in beneficiary management: The array length doesn't reflect the actual number of valid beneficiaries

The issue is rated as medium severity because:

  • It can lead to permanent fund loss under specific conditions

  • It requires particular actions (adding zero address or using the removeBeneficiary function)

  • It fundamentally breaks the inheritance distribution mechanism

Tools Used

Manual code review and Foundry testing

Proof of Concept

  1. Add the following test to InheritanceManager.t.sol

function test_zeroAddressBeneficiaryFundLoss() public {
address user2 = makeAddr("user2");
​
vm.startPrank(owner);
// Add a valid beneficiary
im.addBeneficiery(user1);
// Add the zero address as a beneficiary
im.addBeneficiery(address(0));
// Add another valid beneficiary
im.addBeneficiery(user2);
​
// Fund the contract
vm.deal(address(im), 3 ether);
vm.stopPrank();
​
// Wait for inheritance timelock to expire
vm.warp(block.timestamp + 91 days);
​
// Trigger inheritance
vm.prank(user1);
im.inherit();
​
// Try to withdraw funds - this will attempt to send 1 ETH to address(0)
vm.prank(user1);
// This may revert or burn 1 ETH depending on the environment
try im.withdrawInheritedFunds(address(0)) {
// If it succeeds, check that 1 ETH was sent to address(0) (effectively burned)
assertEq(address(0).balance, 1 ether);
assertEq(user1.balance, 1 ether);
assertEq(user2.balance, 1 ether);
} catch {
// If it reverts, no one gets their funds
assertEq(address(im).balance, 3 ether);
assertEq(user1.balance, 0);
assertEq(user2.balance, 0);
}
}

  1. Run the test

$ forge test --mt test_zeroAddressBeneficiaryFundLoss

Recommendations

  1. Add zero address validation in the addBeneficiery function:

function addBeneficiery(address _beneficiary) external onlyOwner {
require(_beneficiary != address(0), "InheritanceManager: zero address not allowed");
beneficiaries.push(_beneficiary);
_setDeadline();
}

  1. Implement proper array element removal in removeBeneficiary using swap-and-pop:

function removeBeneficiary(address _beneficiary) external onlyOwner {
bool found = false;
uint256 indexToRemove = 0;
​
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] == _beneficiary) {
indexToRemove = i;
found = true;
break;
}
}
​
require(found, "InheritanceManager: beneficiary not found");
​
// Swap with the last element and pop (actually removes the element)
beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
beneficiaries.pop();
​
_setDeadline();
}

  1. Add a check in withdrawInheritedFunds to skip zero addresses:

function withdrawInheritedFunds(address _asset) external {
// ...
uint256 divisor = 0;
// First count valid beneficiaries
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] != address(0)) {
divisor++;
}
}
​
require(divisor > 0, "InheritanceManager: no valid beneficiaries");
​
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
​
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] != address(0)) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "transfer failed");
}
}
} else {
// Similar changes for token transfers
// ...
}
}
Updates

Lead Judging Commences

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