Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Invalid

withdrawInheritedFunds does not handle failures

Summary

If either an ETH or an ERC20 transfer in withdrawInheritedFunds fails, the whole function will revert. This can lock all funds within the contract.

Vulnerability Details

If a beneficiary is a contract which reverts in the receive function this will cause withdrawInheritedFunds to revert.

For instance with this contract:

contract MaliciousBeneficiary{
receive() external payable {
revert("No one gonna get a penny!");
}
}

This test will fail:

function testContractCanSendETHWithMaliciousBeneficiary() public {
vm.deal(address(im), 10 ether);
vm.startPrank(owner);
im.addBeneficiery(aladar);
im.addBeneficiery(bloki);
im.addBeneficiery(address(mailiciousBeneficiary));
im.removeBeneficiary(bloki);
vm.stopPrank();
vm.warp(1 + 90 days);
vm.startPrank(aladar);
im.inherit();
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
}

Impact

A malicious user can prevent other users from getting their inheritance permanently (on the price of loosin his/her inheritance as well). This breaks the intended functionality of the contract.

Tools Used

Manual review, foundry tests.

Recommendations

If transfer fails to one user contnue sending ETH to other users. Design some business logic that handles the remaining ETH in such a case (e.g. divide it among other beneficiaries or allow beneficiaries to claim their shares one by one, etc).

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.