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

Beneficiary Management Flaws Leading to Fund Loss

dumbbutt0

Summary

The removeBeneficiary function in InheritanceManager.sol has a flaw that causes funds to be lost. When a beneficiary is removed, their slot in the beneficiaries array is set to address(0) without shortening the array. Later, when withdrawInheritedFunds distributes funds, it sends a portion to address(0), effectively burning those assets.

Vulnerability Details

How the Exploit Works

When a beneficiary is removed, the array retains its length, and the removed slot becomes address(0). The withdrawInheritedFunds function then splits the contract’s balance among all array entries, including address(0), causing a portion of the funds to be unrecoverable.

Step-by-Step Exploit Execution

  1. Setup:

    • The contract is deployed with two beneficiaries: beneficiary1 and beneficiary2.

    • The owner calls removeBeneficiary(beneficiary1), setting its slot to address(0).

  2. Inheritance:

    • After 90 days of inactivity, beneficiary2 calls inherit() to take ownership.

  3. Fund Withdrawal:

    • The contract receives 1 ETH (e.g., via vm.deal).

    • beneficiary2 calls withdrawInheritedFunds(address(0)) to withdraw ETH.

    • The function splits 1 ETH between two “beneficiaries” (including address(0)):

      • address(0) gets 0.5 ETH (lost).

      • beneficiary2 gets 0.5 ETH.

  4. Result:

    • Half the funds (0.5 ETH) are burned, and beneficiary2 receives only half of what they should.

Impact

Funds (e.g., ETH) are sent to address(0) and lost forever, reducing the amount beneficiaries receive.

  • Root Cause: The removeBeneficiary function doesn’t properly manage the array, leaving address(0) as a “valid” beneficiary that receives funds.

Tools Used

slither, my own scanner, AI and foundry

Proof of Concept

function test_RemoveBeneficiaryFundsLoss() public {
vm.startPrank(owner);
manager = new InheritanceManager();
manager.addBeneficiery(beneficiary1);
manager.addBeneficiery(beneficiary2);
vm.stopPrank();
​
vm.prank(owner);
manager.removeBeneficiary(beneficiary1); // Sets slot to address(0)
vm.warp(block.timestamp + 90 days + 1);
vm.prank(beneficiary2);
manager.inherit();
vm.deal(address(manager), 1 ether); // Fund the contract
uint256 initialBalance = beneficiary2.balance;
vm.prank(beneficiary2);
manager.withdrawInheritedFunds(address(0));
assertEq(beneficiary2.balance - initialBalance, 0.5 ether); // Only half received
assertEq(address(manager).balance, 0); // All funds distributed
}

Recommendations

This bug results in permanent loss of assets, undermining the contract’s purpose of fairly distributing funds to beneficiaries. The more beneficiaries removed, the worse the loss becomes.

Issue

The removeBeneficiary function sets a beneficiary’s slot to address(0) without reducing the array length. This causes withdrawInheritedFunds to mistakenly send funds to address(0), resulting in lost assets.

Fix

Update removeBeneficiary to properly remove entries by swapping the target with the last element and reducing the array size:

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
if (indexToRemove >= beneficiaries.length) revert BeneficiaryNotFound();
beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
beneficiaries.pop();
}

Explanation

  • Change: Replaced setting the slot to address(0) with a swap-and-pop approach.

  • Why It Works: Swapping the target beneficiary with the last element and calling pop() removes the entry cleanly, keeping the array free of invalid addresses.

  • Best Practice: When removing items from dynamic arrays, adjust the length to maintain data integrity and prevent unintended behavior.

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.