Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

`InheritanceManager:inherit` leads to loss of control of the manager

Summary

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

In this line, we check if the length is equal to 1, but we don't check if the msg.sender is actually beneficiaries[0], and we give control to the msg.sender

Impact

Anyone can gain control in case the deadline has passed and only one beneficiary is set

Recommendations

Due to specifications:

  • The owner lists his backup wallet as the only beneficiary.

  • After 90 days, the owner can reclaim his funds via InheritanceManager::inherit().

We should check if the caller is actually the beneficiary.

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
- if (beneficiaries.length == 1){
+ if (beneficiaries.length == 1 && beneficiaries[0] == msg.sender){
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
Updates

Lead Judging Commences

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