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

[H-5] Missing Reentrancy Protection in withdrawInheritedFunds Function

d33p

Summary

The withdrawInheritedFunds function in the InheritanceManager contract lacks reentrancy protection despite making multiple external calls that could be exploited by malicious contracts. This oversight contradicts the security pattern applied to other functions in the contract and could potentially lead to fund drainage.

Vulnerability Details

The function makes external calls to transfer ETH and ERC20 tokens without the nonReentrant modifier that is consistently applied to other sensitive functions in the contract:

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 function is particularly vulnerable to reentrancy attacks because:

  1. It uses low-level call to transfer ETH, which can trigger arbitrary code execution in the recipient

  2. It makes multiple transfers in a loop, which could be exploited if one of the beneficiaries is a malicious contract

  3. It calculates the distribution amount once at the beginning based on the current balance

  4. No state is updated between transfers to prevent repeated withdrawals

Impact

If one of the beneficiaries is a malicious contract, it could exploit this vulnerability to:

  1. Reenter the withdrawInheritedFunds function before the first call completes

  2. Drain additional funds beyond their fair share

  3. Disrupt the inheritance distribution process

The severity is high because:

  • It directly affects the core fund distribution mechanism

  • It could lead to loss of funds beyond intended amounts

  • It contradicts the security pattern applied to other functions in the contract

Tools Used

Manual code review

Recommended Mitigation

Add the nonReentrant modifier (make sure to first fix the issue in that modifier as reported) to the function to prevent reentrancy attacks:

function withdrawInheritedFunds(address _asset) external nonReentrant {
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
);
}
}
}

For additional security, consider also implementing a pull pattern as suggested in the beneficiary withdrawal distribution issue, which would inherently provide better protection against reentrancy by allowing each beneficiary to withdraw their funds independently.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.