Inheritable Smart Contract Wallet

Summary

The removeBeneficiary() function has a security flaw in how it removes beneficiaries from the beneficiaries array. Instead of properly removing an element and adjusting the array's length, it uses the delete keyword, which sets the specified element to address(0) (the null address) without reducing the array size. This leads to several serious problems, including gas inefficiency, potential loss of funds, and incorrect logic in inheritance distribution.

Vulnerability Details

In the current implementation, the function identifies the index of the beneficiary to remove and uses delete to set that element to address(0). However, this does not shift the remaining elements or decrease the array's length, leaving gaps in the array.

Vulnerable Code:

Impact

• Gas Costs: Iterating over an array with null entries wastes computational resources, increasing transaction fees.

• Fund Loss: Sending funds to address(0) during inheritance distribution can result in permanent loss or failed transactions, preventing beneficiaries from accessing their shares.

• Logic Errors: Incorrect array length misleads functions that depend on the number of beneficiaries, disrupting the contract's intended behavior.

Proof of Concept (PoC)

1. The beneficiaries array starts as [address1, address2, address3].

2. The owner calls removeBeneficiary(address2).

3. The array becomes [address1, address(0), address3], with length = 3.

4. When withdrawInheritedFunds() is called to distribute funds, it attempts to send funds to address(0), causing the transaction to revert and blocking all beneficiaries from receiving their inheritance.

Tools Used

• Manual review

Recommendations

Modify the function to properly remove the beneficiary by shifting elements, ensuring no empty slots are left in the array: