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

In `InheritanceManager:removeBeneficiary` removing a beneficiary leaves an address(0) in the array, which disrupts multiple ERC20 transfer functions (buyOutEstateNFT, withdrawInheritedFunds) as transfers to address(0) will revert.

vincent71399

Description: In InheritanceManager:removeBeneficiary, delete beneficiaries[indexToRemove] will set beneficiaries[indexToRemove] to address(0),
but the array length will not be reduced. when other functions (buyOutEstateNFT, withdrawInheritedFunds) loop through the
beneficiaries array, it will transfer assets to address(0), which will revert the transaction.

function removeBeneficiary(address _beneficiary) external onlyOwner {
...
@> delete beneficiaries[indexToRemove];
...
}

Impact: This issue disrupts the contract's functionality, causing it to always revert if the owner removed a beneficiary before.

Proof of Concept: Add the following test and run it

function test_removeBeneficiaryBreakBuyOutEstate() public {
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
im.addBeneficiery(otherUser);
im.removeBeneficiary(otherUser);
im.createEstateNFT("my estate", 2e18, address(usdc));
vm.stopPrank();
vm.warp(im.getDeadline());
im.inherit();
usdc.mint(beneficiary1, 1e18);
vm.startPrank(beneficiary1);
usdc.approve(address(im), 1e18);
vm.expectRevert(abi.encodeWithSelector(
ERC20InvalidReceiver.selector,
address(0)
));
im.buyOutEstateNFT(1); // will cause revert here
vm.stopPrank();
}

Recommended Mitigation:
should swap the last element with the element to remove and then pop the last element.

function removeBeneficiary(address _beneficiary) external onlyOwner {
...
- delete beneficiaries[indexToRemove];
+ beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
+ beneficiaries.pop();
...
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect removal from beneficiary list causes funds to be send to 0 address

Support

FAQs

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

Give us feedback!