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

removeBeneficiary doesn't handle non existing addresses well

nypet

Summary

The removeBeneficiary doesn't handle non existing addresses well, if a non-existing address is provided, it will remove the first added beneficiary.

Vulnerability Details

The removeBeneficiary(address beneficiary) external onlyOwner function is intended to remove a beneficiary. If the beneficiary parameter of the function is not an address previously added to the beneficiaries, it will remove the first added beneficiery, because in such a case function _getBeneficiaryIndex(address _beneficiary) public view returns (uint256 _index) will return 0.

The following test demonstrates the issue:

function testCannotRemoveNonExistingBeneficiary() public {
weth.mint(address(im), 12000);
vm.startPrank(owner);
im.addBeneficiery(aladar);
im.addBeneficiery(bloki);
im.addBeneficiery(address(mailiciousBeneficiary));
uint256 idx = im._getBeneficiaryIndex(geza);
//idx should be a negative number, since it is uint256 at the moment no assertion can be written for it
console.log("_getBeneficiaryIndex result: %d", idx);
im.removeBeneficiary(geza);
vm.stopPrank();
vm.warp(1 + 90 days);
vm.startPrank(aladar);
im.inherit();
im.withdrawInheritedFunds(address(weth));
vm.stopPrank();
}

Impact

If the owner out of mistake tries to remove an address not added to the beneficiaries it will remove the first beneficary. (Because of a vulnerability described in another finding this will make it impossible to regain funds in the contract)

Tools Used

Manual review, foundry test.

Recommendations

If address is not found _getBeneficiaryIndexshould return -1, and remove should do nothing in that case.

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.