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

Missing Access Control in withdrawInheritedFunds() Function

dappscout

Summary

The withdrawInheritedFunds() function in the InheritanceManager contract lacks the onlyBeneficiaryWithIsInherited() modifier, which is essential for restricting access to authorized beneficiaries only.

Vulnerability Details

The withdrawInheritedFunds() function in the InheritanceManager contract lacks the onlyBeneficiaryWithIsInherited() modifier, which is essential for restricting access to authorized beneficiaries only. This omission creates a significant security vulnerability that could allow unauthorized users to withdraw funds from 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);
}
}
}

PoC:

function test_unauthorizedWithdrawal() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
address attacker = makeAddr("attacker");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 9e18);
vm.warp(1 + 90 days);
// User1 triggers inheritance
vm.prank(user1);
im.inherit();
// Attacker (not a beneficiary) can still call withdrawInheritedFunds
vm.prank(attacker);
im.withdrawInheritedFunds(address(0));
// Funds are distributed to beneficiaries despite being triggered by an attacker
assertEq(user1.balance, 3e18);
assertEq(user2.balance, 3e18);
assertEq(user3.balance, 3e18);
}

Impact

The absence of proper access control allows any external address to call this function once the contract has been inherited (isInherited = true), so this couse:

  1. Unauthorized Fund Withdrawal: Any address can trigger the distribution of funds, not just the legitimate beneficiaries.

  2. Premature Distribution: A malicious actor could trigger distribution before beneficiaries are ready or have coordinated their actions

  3. Potential Fund Loss: Since the function distributes to all beneficiaries in the array (including address(0) entries), premature triggering could result in suboptimal distribution or permanent loss of funds.

  4. Disruption of Inheritance Process: The inheritance process is designed to be controlled by beneficiaries, but this vulnerability undermines that control.

Tools Used

Foundry

Recommendations

Add the onlyBeneficiaryWithIsInherited() modifier to the function withdrawInheritedFund(). Consider implementing a pull pattern where each beneficiary claims their share individually.

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.