Inheritable Smart Contract Wallet

Summary

The InheritanceManager contract contains a vulnerability in the way it removes beneficiaries from the beneficiaries array. When a beneficiary is removed, their entry in the array is deleted but not properly replaced, leaving a 0x0 (zero address) in the array. This can lead to incorrect fund distribution.

Vulnerability Details

The removeBeneficiary() function in InheritanceManager removes a beneficiary by using delete beneficiaries[indexToRemove];, which does not shift or replace elements in the array. Instead, it leaves a 0x0 entry in the array, which can lead to multiple issues:

1. Funds Sent to Zero Address

   • When assets are withdrawn in withdrawInheritedFunds(), the function distributes funds among all beneficiaries, including the 0x0 address.

   • This results in assets being permanently lost.

2. Incorrect Asset Distribution

   • The function assumes that all entries in the beneficiaries array are valid addresses, leading to incorrect calculations for share distribution.

   • Beneficiaries may receive less than their entitled amount due to the presence of an invalid entry.

Impact

1. Unintended burning of funds by sending them to the zero address when withdrawing assets in InheritanceManager::withdrawInheritedFunds()

2. Incorrect calculation of asset shares in InheritanceManager::withdrawInheritedFunds() leading to beneficiaries receiving less than what they should.

Tools Used

1. Manual review

Recommendations

You can overwrite the beneficiary to be removed with the last element in the array and pop the last item from the array, this ensures that you will not have zero address in the beneficiaries array.