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

Funds Burned, and DOS on ERC20 tokens withdrawals Due to Improper Beneficiary Removal

derastephh

Summary

The contract fails to properly update the beneficiary list when a beneficiary is removed. As a result, the inheritance distribution calculation includes removed beneficiaries, causing a portion of the funds to be sent to address(0), effectively burning them, and also and causing reverts to occur for ERC20 tokens withdrawal.

Vulnerability Details

Issue in inheritanceManager::withdrawInheritedFunds Function

uint256 divisor = beneficiaries.length;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;

The function determines amountPerBeneficiary using beneficiaries.length, which still includes removed beneficiaries. Since inheritanceManager::removeBeneficiary only deletes an entry but does not reduce the array size, the divisor remains unchanged. The contract then attempts to send funds to removed beneficiaries, resulting in ETH being sent to address(0), effectively burning it, and causing reverts to occur for ERC20 tokens withdrawal.

Proof Of Concept

Paste the following test in inheritanceManagerTest.t.sol file.

function testETHGetsBurned() public {
vm.deal(address(im), 12e18);
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
address user4 = makeAddr("user4");
address user5 = makeAddr("user5");
vm.startPrank(owner);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.addBeneficiery(user4);
im.addBeneficiery(user5);
im.removeBeneficiary(user2);
im.removeBeneficiary(user3);
vm.stopPrank();
console.log("Beneficiaries length:", im.getBeneficiariesLength()); // still remains 4 despite removal of two beneficiaries
​
vm.warp(1 + 90 days);
console.log("Contract Balance before funds are inherited:", address(im).balance); // 12e18
​
vm.startPrank(user4);
im.inherit();
im.withdrawInheritedFunds(address(0));
console.log("user4 balance", user4.balance); // 3e18 instead of 6e18
console.log("user5 balance", user5.balance); // 3e18 instead of 6e18
console.log("Contract Balance after funds are inherited:", address(im).balance); // 0
​
vm.expectRevert();
im.withdrawInheritedFunds(address(usdc)); // reverts
vm.stopPrank();
}

POC Explanation

  1. The contract originally holds 12 ETH.

  2. Owner adds 4 beneficiaries, but later removed 2 beneficiaries.

  3. Since two beneficiaries were removed but still counted in the divisor, the inheritanceManager::withdrawInheritedFunds function incorrectly assumes there are 4 recipients.

  4. Only 6 ETH is properly distributed to active beneficiaries.

  5. The remaining 6 ETH is sent to address(0), effectively burning it.

  6. ERC20 tokens reverts, thereby causing Denial of Service.

Impact

  • Some ETH meant for active beneficiaries is lost forever.

  • Beneficiaries receive less than their rightful share.

  • Unintended burning of funds that should have been inherited.

  • ERC20 tokens reverts, thereby causing Denial of Service.

Tools Used

  • Foundry

Recommendations

  • Modify inheritanceManager::removeBeneficiary to correctly update the array:

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

This ensures the array size properly reflects the number of active beneficiaries.

Updates

Lead Judging Commences

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