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

Missing access control on function `InheritanceManager::inherit()` could lead to loss of contract ownership and funds

vladich0x

Summary

The function InheritanceManager::inherit() has no access control, so any malicious actor could compromise contracts behavior, leading ot loss of wallet ownership and funds

Vulnerability Details

The function InheritanceManager::inherit() is set to be an external function, and according to the natspec and overall project documentation, only the beneficiaries should be able to call it after the owner has not used the wallet for 90 days.

function inherit() external {
// missing access control
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

  • Personal backup scenario: if malicios actor calls the funtion inherit() before the specified beneficiary, he will gain the ownership of the wallet.

  • Inheritance scenario: if malicios actor calls the function before specified beneficiaries, he will cause an authorized state change into inherited and will enable additional logic to be called InheritanceManager::withdrawInheritedFunds.

Tools Used

Manual review

Recommendations

Add access control to function InheritanceManager::inherit(), allowing only beneficiaries to call it.

+ // allows only beneficiaries to call the funtcion
+ modifier onlyBeneficiary() {
+ bool isBeneficiary = false;
+ for (uint256 i = 0; i < beneficiaries.length; i++) {
+ if (msg.sender == beneficiaries[i]) {
+ isBeneficiary = true;
+ break;
+ }
+ }
+ require(isBeneficiary, "Caller is not a beneficiary");
+ _;
+ }
​
- function inherit() external {
+ function inherit() external onlyBeneficiary { // access controll modifier
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
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.