Submission Details
Severity:
`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 9 months ago
Submission Judgement Published Assigned finding tags:
Inherit depends on msg.sender so anyone can claim the contract
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.