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

[H-1] Inheritance Logic Vulnerability Allows Unauthorized Ownership Claims

d33p

Summary

The InheritanceManager contract has a critical vulnerability in its inherit() function where anyone can call it when the deadline passes. This allows unauthorized users to claim ownership if there's only one beneficiary, regardless of whether the caller is in the beneficiary list.

Vulnerability Details

The inherit() function lacks proper access control and allows anyone to trigger the inheritance process:

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

This implementation has a critical issue: it doesn't verify that msg.sender is actually a beneficiary before transferring ownership. When there's only one beneficiary, it transfers ownership to whoever calls the function first, which could be anyone.

Impact

This vulnerability could lead to:

  1. Unauthorized users claiming ownership of the contract

  2. Complete theft of all assets controlled by the contract

  3. Bypassing the intended inheritance process

  4. Legitimate beneficiaries being unable to claim their inheritance

The severity is high because it allows complete takeover of the contract and all its assets by unauthorized users, fundamentally breaking the core purpose of the contract.

Tools Used

Manual code review

Recommendations

Implement proper access control in the inherit() function:

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

Alternatively, consider using a mapping to track beneficiaries for more efficient access control.

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.