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

Security Analysis: Missing Access Control for `withdrawInheritedFunds`

0x68

Summary

The withdrawInheritedFunds function in the InheritanceManager contract lacks proper access control. While it checks if inheritance has been triggered via the isInherited flag, it does not verify that the caller is actually a beneficiary. This allows any external address to initiate fund distribution once the inheritance process has been activated.

Vulnerability Details

The vulnerable function is implemented as follows:

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 only checks for isInherited status but fails to verify that msg.sender is part of the beneficiaries array. This contrasts with other inheritance-related functions like buyOutEstateNFT and appointTrustee, which correctly use the onlyBeneficiaryWithIsInherited modifier to restrict access.

Impact

The impact of this vulnerability is high:

  1. Unauthorized Fund Distribution: Any external actor can trigger the distribution of funds (ETH or ERC20 tokens) once inheritance is activated, potentially at a time that is disadvantageous to the beneficiaries.

  2. Forced Execution: Malicious actors could force the execution of this function before beneficiaries have had time to prepare (e.g., setting up wallets or making other arrangements).

  3. DOS Attack: A malicious actor could repeatedly call this function with different token addresses, causing unnecessary gas costs for beneficiaries and potentially disrupting legitimate withdrawal attempts.

  4. Timing Manipulation: A front-runner could observe a beneficiary's attempt to call this function and execute it first, potentially manipulating the timing for their own benefit.

Tools Used

Manual code review.

Recommendations

  1. Add appropriate access control by using the existing onlyBeneficiaryWithIsInherited modifier:

function withdrawInheritedFunds(address _asset) external onlyBeneficiaryWithIsInherited {
// Existing function body...
}

  1. Alternatively, implement explicit beneficiary verification:

function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
// Verify caller is a beneficiary
bool isBeneficiary = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Caller is not a beneficiary");
// Existing distribution logic...
}

  1. Consider adding a tracker to prevent multiple withdrawals of the same asset:

// Add this mapping to state variables
mapping(address => bool) public assetWithdrawn;
​
function withdrawInheritedFunds(address _asset) external onlyBeneficiaryWithIsInherited {
if (!isInherited) {
revert NotYetInherited();
}
require(!assetWithdrawn[_asset], "Asset already withdrawn");
// Existing distribution logic...
assetWithdrawn[_asset] = true;
}

  1. Add events to track withdrawals:

event FundsWithdrawn(address asset, uint256 totalAmount, uint256 perBeneficiaryAmount);
​
function withdrawInheritedFunds(address _asset) external onlyBeneficiaryWithIsInherited {
// Function implementation...
emit FundsWithdrawn(_asset, totalAmount, amountPerBeneficiary);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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