Inheritable Smart Contract Wallet

Summary:

Hi,

I found out a potential vulnerability in which the contract owner removes beneficiary in the function 'removeBeneficiary' in the incorrect way i.e. without proper checking of the beneficiary address.

Vulnerability Details:

The key details of this potential vulnerability can be given as follows:

In the function 'removeBeneficiary' of contract 'Inheritancemanager.sol', we can see in the line 165, delete beneficiaries[indexToRemove] removes a beneficiary, sets the element at indexToRemove to address(0) i.e. no adjustment in array length or order. Meaning that Funds in 'withdrawInheritedFunds' will still be divided by the original beneficiaries.length, potentially sending funds to address(0) and eventually burning them.

Also, the loop in function 'buyOutEstateNFT' may attempt to send funds to address(0) as it collects _beneficiary address from this function can cause unintended behavior or reverts.

Impact:

1. Loss of funds / Invalid beneficiary states leads to contract unsuability.

2. The presence of address(0) in the array could allow malicious actors to exploit edge cases in inheritance logic.

Tools Used:

Manual Code Analysis + VS Code

Recommendations:

Implement proper array shifting of the elements in array length (code given below) or use Openzeppelin's EnumerableSet for more efficiency. (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/structs/EnumerableSet.sol)