Inheritable Smart Contract Wallet

Finding description and impact

The InheritanceManager contract contains a vulnerability in the addBeneficiery function that allows duplicate beneficiary entries. The core issue stems from the lack of validation before adding a new beneficiary to the array, enabling the same address to be added multiple times.

• addBeneficiery() function

The impact is that:

• A beneficiary added multiple times will receive multiple shares during fund distribution

• The inheritance distribution mechanism becomes unfair and unpredictable

• In the withdrawInheritedFunds function, assets are divided by beneficiaries.length, so duplicate entries receive proportionally more funds

• Gas costs for operations that iterate through the beneficiaries array will increase unnecessarily

Proof of Concept

The following code segment demonstrates the vulnerability:

Consider this scenario:

1. Owner adds Alice as a beneficiary: addBeneficiery(aliceAddress)

2. Owner adds Bob as a beneficiary: addBeneficiery(bobAddress)

3. Owner accidentally (or deliberately) adds Alice again: addBeneficiery(aliceAddress)

4. When funds are distributed in withdrawInheritedFunds, the division would be:

   uint256 divisor = beneficiaries.length; // This equals 3 instead of 2

   uint256 amountPerBeneficiary = assetAmountAvailable / divisor;

5. Alice would receive 2 * amountPerBeneficiary while Bob would only receive amountPerBeneficiary, despite them supposedly having equal shares

This issue affects other functions as well:

• buyOutEstateNFT would send duplicate payments to beneficiaries listed multiple times

• inherit function's behavior could be unpredictable if there are duplicates in the array

Recommended mitigation steps

To fix this issue, implement a duplicate check in the addBeneficiery function:

Alternatively, a more gas-efficient approach using a mapping:

Tools Used

• Manual Code Review

• Logical Analysis