Inheritable Smart Contract Wallet

Summary

The InheritanceManager::removeBeneficiary function in InheritanceManager is intended to remove a specified beneficiary from the beneficiaries array. However, the implementation incorrectly uses delete, which only resets the value at the given index to address(0) without reducing the array length. This results in empty slots in the array, leading to incorrect beneficiary calculations, unnecessary gas consumption, and potential unintended behavior when iterating over the list.

Vulnerability Details

Issue Explanation

When delete beneficiaries[indexToRemove] is executed, the element at that index is replaced with address(0), but the overall length of the array remains unchanged. This creates an empty slot that still occupies space in storage, which can cause issues when iterating over beneficiaries.

Functions that rely on the correct number of beneficiaries, such as withdrawInheritedFunds, may incorrectly count address(0) as a valid entry, leading to incorrect fund distribution. Additionally, iterating over an array with empty slots increases gas costs, as unnecessary checks need to be performed to skip address(0).

Impact

The use of delete causes the array to retain an incorrect structure, leading to inefficient gas usage and potential miscalculations in fund distribution. If withdrawInheritedFunds splits funds equally among beneficiaries.length, empty slots could cause either an incorrect division of funds or failed transactions. The increased gas costs from iterating over a larger-than-necessary array further degrades the contract’s efficiency.

Tools Used

• Manual Code Review

Recommendations

Instead of using delete, which leaves gaps, the function should use the swap-and-pop method to efficiently remove the beneficiary while maintaining a contiguous array.

Fixed Code