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

[M-4] Possible Beneficiary Withdrawal Distribution Disruption

d33p

Summary

The withdrawInheritedFunds function in the InheritanceManager contract uses a push pattern for distributing funds to beneficiaries. If one beneficiary's transfer reverts (e.g., due to a malicious contract or gas limit), all subsequent transfers will fail, potentially blocking the distribution process.

Vulnerability Details

The withdrawInheritedFunds function distributes funds to all beneficiaries in a single transaction:

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
);
}
}
}

This implementation has several issues:

  1. It uses a push pattern where the contract sends funds to each beneficiary

  2. If any beneficiary's transfer fails (e.g., due to a malicious contract that reverts), the entire function reverts

  3. This could block all beneficiaries from receiving their funds

  4. A malicious beneficiary could intentionally block the distribution

Impact

This vulnerability could lead to:

  1. Permanent blocking of fund distribution if one beneficiary is malicious

  2. Denial of service for legitimate beneficiaries

  3. Funds being locked in the contract indefinitely

  4. Requiring contract redeployment or complex workarounds

The severity is medium because while it could lead to temporary fund locking, it doesn't directly expose funds to theft and requires a malicious beneficiary.

Tools Used

Manual code review

Recommendations

Implement a pull pattern instead of a push pattern for fund distribution:

// Track each beneficiary's share
mapping(address => uint256) public beneficiaryShares;
bool public sharesCalculated;
​
// Calculate shares once
function calculateShares(address _asset) external {
require(isInherited, "Not inherited yet");
require(!sharesCalculated, "Shares already calculated");
if (_asset == address(0)) {
uint256 ethAmount = address(this).balance / beneficiaries.length;
for (uint256 i = 0; i < beneficiaries.length; i++) {
beneficiaryShares[beneficiaries[i]] = ethAmount;
}
} else {
uint256 tokenAmount = IERC20(_asset).balanceOf(address(this)) / beneficiaries.length;
for (uint256 i = 0; i < beneficiaries.length; i++) {
beneficiaryShares[beneficiaries[i]] = tokenAmount;
}
}
sharesCalculated = true;
emit SharesCalculated(_asset);
}
​
// Allow each beneficiary to withdraw their share
function withdrawShare(address _asset) external {
require(sharesCalculated, "Shares not calculated yet");
uint256 share = beneficiaryShares[msg.sender];
require(share > 0, "No share available or already withdrawn");
beneficiaryShares[msg.sender] = 0;
if (_asset == address(0)) {
payable(msg.sender).transfer(share);
} else {
IERC20(_asset).safeTransfer(msg.sender, share);
}
emit ShareWithdrawn(msg.sender, _asset, share);
}

This pattern allows each beneficiary to withdraw their funds independently, preventing a single malicious beneficiary from blocking the entire distribution process.

Updates

Lead Judging Commences

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