Inheritable Smart Contract Wallet

Summary

The _getBeneficiaryIndex function in the InheritanceManager contract is responsible for retrieving the index of a given beneficiary within the beneficiaries array. However, the function currently has a vulnerability that could lead to incorrect deletions or unintended behavior if the beneficiary is not found.

Vulnerability Details

• The function attempts to return an index for a given beneficiary.

• If the beneficiary does not exist in the beneficiaries array, the function does not return a valid index but instead leaves _index uninitialized.

• In Solidity, an uninitialized uint256 defaults to 0. This means that if the function is used incorrectly (without proper validation), it could lead to unintended consequences, such as deleting the wrong beneficiary from the array.

• Although the function does include a check (if (!found) { revert("Beneficiary not found"); }), it still declares _index as a return variable, which may cause confusion and incorrect usage.

Impact

• Incorrect deletions: If a non-existent beneficiary is queried and the returned value is mistakenly assumed to be valid (0), the first beneficiary in the list may be incorrectly removed.

• Unexpected behavior: Other functions that rely on this function may process incorrect data due to the lack of a safer return type.

• Potential security risk: This vulnerability could be exploited if an attacker manipulates function calls that rely on _getBeneficiaryIndex.

Tools Used

Manual Review

The Vulnerable Code

Recommendations

To mitigate this issue, the _getBeneficiaryIndex function should be updated to ensure that it does not return an uninitialized value. The best practice is to revert if the beneficiary is not found before attempting to return any value.

Key Fixes

1. Removed the found boolean: Instead of tracking whether the beneficiary is found, the function now immediately returns the index when a match is found.

2. Ensured function always reverts if the beneficiary is not found: The function now reverts before attempting to return an index, preventing potential misuse of an uninitialized variable.