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

Adding duplicate beneficiaries with `InheritanceManager::addBeneficiery`

0xdonchev

Summary

The owner of InheritanceManager can make a mistake and add the same beneficiery address more than once.

Vulnerability Details

There are no any checks that can prevent adding a duplicate addresses to the InheritanceManager::beneficieries with InheritanceManage::addBeneficiery.

function addBeneficiery(address _beneficiary) external onlyOwner {
beneficiaries.push(_beneficiary);
_setDeadline();
}

Impact

Having the same address of a beneficiary more than once in the InheritanceManager::beneficiaries array can cause huge issues for withdrawing assets. When InheritanceManager::withdrawInheritedFunds is called, the function devides the assets by the length of beneficiaries[]. If there are duplicates, the same address will unfairly increase its inheritance at the expense of others.

I added a function to get the addresses of the beneficiaries in the InheritanceManager::beneficiaries array to help me with the PoC. Place the function into InheritanceManager.sol.

function _getBeneficiariesAddress(uint256 index) public view returns (address) {
return beneficiaries[index];
}

Place the following into InheritanceManagerTest.t.sol.

function test_addDuplicateBeneficiaries() public {
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user1);
vm.stopPrank();
assertEq(im._getBeneficiariesAddress(0), user1);
assertEq(im._getBeneficiariesAddress(1), user1);
}

Tools Used

-manual analysis

Recommendations

Update InheritanceManage::addBeneficiery to check for duplicates in the array.

function addBeneficiery(address _beneficiary) external onlyOwner {
+ for (uint256 i = 0; i < beneficiaries.length; i++) {
+ require(beneficiaries[i] != _beneficiary, "Beneficiary already added");
+ }
beneficiaries.push(_beneficiary);
_setDeadline();
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 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.