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

Duplicate beneficiaries in `addBeneficiery` function breaks equal inheritance distribution

swarecito

Summary

The InheritanceManager contract does not prevent the same address from being added multiple times as a beneficiary. This vulnerability allows certain beneficiaries to receive a larger share of the inheritance than intended, breaking the contract's core assumption of equal fund distribution.

Vulnerability Details

The addBeneficiery function lacks a check to prevent adding duplicate beneficiary addresses:

function addBeneficiery(address _beneficiary) external onlyOwner {
// No check to see if this address already exists in the array
beneficiaries.push(_beneficiary);
_setDeadline();
}

The withdrawInheritedFunds function distributes funds based on the length of the beneficiaries array:

function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
// Funds are divided by the number of entries, not unique addresses
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
// Same issue for ERC20 tokens
// ...
}
}

When a beneficiary appears multiple times in the array, they will receive multiple shares of the inheritance, creating an unfair distribution.

Impact

This vulnerability directly impacts the fairness of inheritance distribution:

  1. Beneficiaries added multiple times receive a proportionally larger share of the inheritance

  2. Other legitimate beneficiaries receive less than their intended share

  3. This could be exploited intentionally to favor certain beneficiaries

  4. Violates the core contract functionality of equal distribution

The impact is rated as medium because:

  • It does not directly lead to fund loss for the contract

  • But it does lead to incorrect fund distribution

  • It breaks the core assumption of equal distribution

Tools Used

Manual code review and Foundry testing

Proof of Concept

  1. Add the following test to InheritanceManager.t.sol

    This test adds user1 twice. We calculate the proportion of funds by dividing the total funds by the number of beneficiaries (4 entries for 3 unique addresses).
    We then assert that user1 gets twice the proportion while other users get one proportion each.

function test_withdrawInheritedFundsEtherDuplicateBeneficiary() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 9e18);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
uint256 proportion = 9e18 / 4;
assertEq(proportion * 2, user1.balance);
assertEq(proportion, user2.balance);
assertEq(proportion, user3.balance);
}

  1. Run the test

forge test --mt test_withdrawInheritedFundsEtherDuplicateBeneficiary

Recommendations

Implement a check to prevent duplicate beneficiary addresses:

function addBeneficiery(address _beneficiary) external onlyOwner {
// Check for duplicates
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] == _beneficiary) {
revert("InheritanceManager: beneficiary already exists");
}
}
​
beneficiaries.push(_beneficiary);
_setDeadline();
}

Alternatively, implement a helper function for reusability:

function _beneficiariesContains(address _beneficiary) internal view returns (bool) {
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] == _beneficiary) {
return true;
}
}
return false;
}
​
function addBeneficiery(address _beneficiary) external onlyOwner {
require(!_beneficiariesContains(_beneficiary), "InheritanceManager: beneficiary already exists");
beneficiaries.push(_beneficiary);
_setDeadline();
}
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!