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

Inheritance does not remove owner privileges

vanshikag

Summary

The InheritanceManager::inherit() flips the isInherited boolean value if there are > 2 beneficiaries in the array. At this point, funds can be distributed between the beneficiaries. Logically, any change in funds or assets at this point should be agreed upon by all beneficiaries. However, the owner still maintains full access and can change contract balances, change beneficiaries, create NFTs and access any onlyOwner functions they like. This should not be possible. This case in inherit() should reset the owner variable back to address(0).

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
// owner should be set to 0 here.
} else {
revert InvalidBeneficiaries();
}
}

Impact

Owner access to functions is not locked after inheritance and this interferes with fund allocation for beneficiaries and also allows for changes in beneficiaries.

Proof Of Concept

Add the following to InheritanceManager.t.sol and run the test:

function test_ownerPrivilegesAfterInheritance() public {
vm.startPrank(owner);
im.addBeneficiery(alice);
im.addBeneficiery(bob);
vm.warp(1 + 90 days);
vm.deal(address(im), 5e18);
im.inherit(); // doesn't matter who calls this.
im.removeBeneficiary(bob); //should not be possible after inheritance.
im.addBeneficiery(owner); // owner access
im.sendETH(5e18, owner);
vm.stopPrank();
assertEq(0, address(im).balance);
}

Expected result:

forge test --mt test_ownerPrivilegesAfterInheritance -vvv
[⠆] Compiling...
[⠑] Compiling 1 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 4.43s
Compiler run successful!
​
Ran 1 test for test/ProofOfConcept.t.sol:InheritanceManagerTest
[PASS] test_ownerPrivilegesAfterInheritance() (gas: 149635)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 13.07ms (1.86ms CPU time)
​

Recommendation

Remove owner privileges after inheritance and set owner variable to address(0).

Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

vanshikag Submitter
6 months ago
0xtimefliez Lead Judge
6 months ago
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.