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

Anyone Can Take Ownership Through inherit() Function

teoslaf

Summary

The inherit() function in InheritanceManager lacks proper validation of msg.sender, allowing any address to take ownership of the contract if there is exactly one beneficiary and the deadline has passed.

Vulnerability Details

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender; // CRITICAL: No validation of msg.sender!
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

Critical issues:

  1. Missing Validation

    • No check if msg.sender is a beneficiary

    • Anyone can call this function

    • Only time-based check is performed

    • Direct ownership transfer to caller

  2. Attack Scenario

    • Wait for deadline to pass

    • Check if there's exactly one beneficiary

    • Call inherit() from any address

    • Instantly become owner

  3. Comparison with Multi-Beneficiary Case

    • Multiple beneficiaries case properly sets isInherited

    • Single beneficiary case directly transfers ownership

    • Inconsistent security model

Impact

CRITICAL - The vulnerability enables:

  1. Complete Contract Takeover

    • Unauthorized ownership transfer

    • Access to all assets

    • Control of all functions

    • Reset of deadline

  2. Inheritance Hijacking

    • Block legitimate beneficiary

    • Steal all contract assets

    • Manipulate contract state

    • Permanent control

Proof of Concept

// 1. Contract has one beneficiary
beneficiaries = [0xlegitimate];
​
// 2. Deadline passes (90 days)
// wait for block.timestamp > deadline
​
// 3. Attacker calls inherit()
function attack() external {
inheritanceManager.inherit(); // Attacker becomes owner!
// Now attacker can:
inheritanceManager.sendETH(address(attacker), address(this).balance);
inheritanceManager.sendERC20(token, amount, address(attacker));
// etc...
}

Recommendations

  1. Add Proper Validation:

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
bool isBeneficiary = false;
for(uint i = 0; i < beneficiaries.length; i++) {
if(msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Not a beneficiary");
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

  1. Consider Unified Inheritance Model:

function inherit() external {
require(block.timestamp >= deadline, "Too soon");
require(isBeneficiary[msg.sender], "Not beneficiary");
isInherited = true;
emit InheritanceTriggered(msg.sender);
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
emit SingleBeneficiaryInherited(msg.sender);
}
}

  1. Add Safety Features:

    • Timelock on ownership transfer

    • Multi-sig for single beneficiary

    • Emergency pause mechanism

    • Proper event emissions

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!