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

[H-9] DoS possibility in `InheritanceManager::withdrawInheritedFunds` when distributing ETH

0xnick

Summary

A malicious beneficiary can easily DoS the contract partially and prevent others from withdrawing ETH from the Smart Wallet.

Vulnerability Details

The withdrawInheritedFunds function distributes ETH to beneficiaries when _asset == address(0) using a push-based approach within a loop:

for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}

When ETH is sent via .call, it triggers the recipient’s receive() or fallback() function (if implemented). If a beneficiary is a contract that reverts during ETH receipt—e.g., by explicitly calling revert(), lacking a receive() function, or running out of gas—the .call returns success = false. The subsequent require(success, "something went wrong") then reverts the entire transaction.

This allows a single malicious or misconfigured beneficiary to prevent all beneficiaries from withdrawing ETH, as the function requires all transfers to succeed. Unlike ERC-20 transfers (which use safeTransfer and handle reverts individually), the ETH dispersal logic lacks resilience against reverting receivers, creating a DoS vulnerability. Such cases are:

  • Not accepting ETH

  • Explicit Revert

Impact

High. The beneficiaries will not be able to withdraw ETH from the contract, leaving the funds locked

Tools Used

  • Manual Review

Recommendations

Implement a pull-payment pattern to mitigate DoS by allowing beneficiaries to withdraw their ETH shares individually, rather than distributing funds in a single transaction. This isolates the impact of a malicious beneficiary to their own withdrawal. Suggested fix:

// Add mapping to track withdrawn amounts
mapping(address => uint256) public ethWithdrawn;
​
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;
bool isBeneficiary = false;
for (uint256 i = 0; i < divisor; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
if (!isBeneficiary) revert("Not a beneficiary");
if (ethWithdrawn[msg.sender] >= amountPerBeneficiary) revert("Already withdrawn");
ethWithdrawn[msg.sender] = amountPerBeneficiary;
(bool success,) = msg.sender.call{value: amountPerBeneficiary}("");
require(success, "ETH transfer failed");
} else {
// Existing ERC-20 logic remains unchanged
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}
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.