Inheritable Smart Contract Wallet

Summary

The removeBeneficiary() function uses delete beneficiaries[index] to remove a beneficiary, which sets the removed beneficiary’s address to address(0) without reducing the array’s length. Downstream functions like withdrawInheritedFunds() and buyOutEstateNFT() calculate amounts based on the full length of the array, creating an inaccurate payout model.

Vulnerability Details

When removeBeneficiary() uses delete, it simply zeroes out the beneficiary’s address but keeps beneficiaries.length the same. Consequently, inheritance distribution or buyout cost is still divided among the total (including the zeroed slot), causing legitimate beneficiaries to receive less than their correct share.

Consider the withdrawInheritedFunds() function, which computes:

If one or more of those beneficiaries is effectively removed but remains in the array as address(0), legitimate beneficiaries are collectively shorted because the division factor is inflated. A similar issue occurs in buyOutEstateNFT()where:

Again, the array length is incorrect, resulting in a miscalculation of how much is owed to real beneficiaries.

Impact

All active beneficiaries receive less than intended whenever funds are distributed or buyouts occur, diluting their rightful allocation. This mistake leads to an underpayment for valid beneficiaries and introduces a mismatch between expected and actual payouts. The error is particularly critical if large sums or valuable assets are involved.

Tools Used

Manual analysis.

Recommendations

Instead of using delete beneficiaries[indexToRemove], remove the beneficiary by swapping it with the last element in the array and then calling .pop(). This maintains the array’s integrity by reducing its length properly, ensuring subsequent calculations correctly reflect the actual number of beneficiaries.