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

No Check for Empty Asset Balance in withdrawInheritedFunds

Description

The withdrawInheritedFunds() function in the InheritanceManager contract fails to verify that there are assets to distribute before proceeding with the distribution process. This can lead to wasteful gas consumption and potential confusion among beneficiaries who receive zero-value transfers.

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

The function retrieves the current balance but does not check if it's zero before proceeding with the distribution calculations and transfers. When amountPerBeneficiary is zero, the function will still execute all the transfer operations, sending zero amounts to each beneficiary.

Impact

While this issue may not directly lead to fund loss, it has several negative consequences:

  1. Gas Waste: Executing the function when there are no assets to distribute wastes gas on unnecessary operations.

  2. Misleading Transactions: Zero-value transfers can confuse beneficiaries, as they appear as transactions but provide no value.

  3. Event Spam: For ERC20 tokens that emit transfer events, these operations create misleading transaction history.

Recommendation

Implement a check for empty balances before proceeding with the distribution:

function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
// Handle ETH withdrawals
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
if (ethAmountAvailable == 0) revert NoAssetsToDistribute();
uint256 divisor = beneficiaries.length;
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, "ETH transfer failed");
}
}
// Handle ERC20 withdrawals
else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
if (assetAmountAvailable == 0) revert NoAssetsToDistribute();
uint256 divisor = beneficiaries.length;
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}

Tools Used

  • Foundry Testing Framework

  • Transaction Trace Analysis

  • Manual Code Review

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.