Summary
The `withdrawInheritedFunds` function uses a push pattern to send funds to the beneficiaries. This is not good because if there's an issue with any one of the transfers (maybe a bad fallback), the entire transaction would revert, DOSing all other beneficiaries.
Vulnerability Details
The issue occurs here:
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
@> for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
@> for (uint256 i = 0; i < divisor; i++) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}
POC
Add the below function and contract to `InheritanceManagerTest.t.sol`:
function testDOS() public {
address zeroIndex = makeAddr("zeroIndex");
address firstIndex = makeAddr("firstIndex");
Beneficiary beneficiary = new Beneficiary();
deal(address(im), 1000 ether);
vm.startPrank(owner);
im.addBeneficiery(zeroIndex);
im.addBeneficiery(firstIndex);
im.addBeneficiery(address(beneficiary));
vm.stopPrank();
vm.warp(block.timestamp + 91 days);
vm.startPrank(zeroIndex);
im.inherit();
vm.expectRevert();
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
}
contract Beneficiary {
fallback() external payable {
revert();
}
}
Impact
Beneficiaries would be unable to get their funds
Tools Used
Manual review, foundry test suite
Recommendations
Use a claiming system where each beneficiary has to manually claim their distributed funds