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

Bad Logic Implementation in InheritanceManager::inherit allows any user to became the contract's owner

medinamv

Summary

The InheritanceManager:inherit function implementation is incorrect for the first case scenario: the owner lost his keys and wants to reclaim this contract from beneficiaries slot 0
In this case, after the 90 days has passed, this function establish the new owner to msg.sender, instead of the only beneficiary. So, if anyone is quicker enough to call this function before the real owner, is going to be able to claim the ownership of the smart contract.

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();
}
}

Impact

A malicious Actor can claim the ownership of the smart contract.

Tools Used

Manual Review

Recommendations

Instead of setting the new owner to msg.sender, change that to the only beneficiary, is this case -> beneficiaries[0].

- owner = msg.sender;
+ owner = beneficiaries[0];

PoC

  1. As the owner, set a unique beneficiary.

  2. Let the 90 days pass.

  3. Call the InheritanceManager::inherit with an attacker account.

  4. Claim the ownership of the contract 😎.

Here is a piece of code that shows this workflow.

function test_inheritCase1IsWrong() public {
address theOneWhoShouldBeTheNewOwner = makeAddr("theOneWhoShouldBeTheNewOwner");
address attacker = makeAddr("attacker");
vm.prank(owner);
im.addBeneficiery(theOneWhoShouldBeTheNewOwner);
vm.warp(1 + 90 days);
vm.prank(attacker);
im.inherit();
address newOwner = im.getOwner();
assert(newOwner != theOneWhoShouldBeTheNewOwner);
assert(newOwner == attacker);
}
Updates

Lead Judging Commences

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

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

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