Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Allowing Duplicate Beneficiaries Leads to Poor User Experience

ssalaudeen10

Summary

The InheritanceManager The InheritanceManager contract permits the same address to be added multiple times as a beneficiary. This is counterintuitive and undermines the purpose of inheritance, which should be distributed among unique beneficiaries only. Allowing duplicates can result in confusion and create an undesirable user

Vulnerability Details

The following test case demonstrates how the issue can occur. This can be added to InheritanceManagerTest.t.sol:

function test_address_can_be_added_multiple_times_as_beneficiary()public{
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user1);
vm.stopPrank();
address getBeneficiaryAtIndexZero= im.getBeneficiaries()[0];
address getBeneficiaryAtIndexOne= im.getBeneficiaries()[1];
vm.assertEq(getBeneficiaryAtIndexZero, user1);
vm.assertEq(getBeneficiaryAtIndexOne, user1);
}

Impact

Multiple entries of the same address dilute the clarity of inheritance distribution, leading to potential issues in fund allocation and increased likelihood of user errors.

Tools Used

Foundry test

Recommendations

To prevent duplicate entries, ensure that an address is validated before being added as a beneficiary. The following changes can be made to InheritanceManager.sol to implement the fix:

contract InheritanceManager is Trustee{
+ error AddressIsNotBeneficiary();
+ error DuplicateBeneficiaryNotAllowed();
​
+ mapping(address => bool) addressAlreadyBeneficiary;
​
function addBeneficiery(address _beneficiary) external onlyOwner {
+ // Check if the address is already a beneficiary
+ if(addressAlreadyBeneficiary[_beneficiary]){
+ revert DuplicateBeneficiaryNotAllowed();
+ }
+ // Mark the address as a beneficiary
+ addressAlreadyBeneficiary[_beneficiary] = true;
​
// Existing logic
​
}
function removeBeneficiary(address _beneficiary) external onlyOwner {
+ if(!addressAlreadyBeneficiary[_beneficiary]){
+ revert AddressIsNotBeneficiary();
+ }
+ // Mark the address as no longer a beneficiary
+ addressAlreadyBeneficiary[_beneficiary] = false;
// Existing logic
}
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!