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

Risk of Out-of-Bands error

0xvadar

Summary

There is a risk of an out-of-bounds error ocurring here because of the "+1"

Vulnerability Details

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length + 1) {
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
_;
}

Impact

This flaw could lead to a DOS attack where the contract is being made to not work by an attacker because you could have a situation where the extra addition of "1" could exceed the set number of beneficiaries of the inheritance

Tools Used

Manual review

Recommendations

This code

```

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length + 1) {
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
_;
}

can be re-written as the code below.

the code below has the following benefits;

The removal of "+1" will prevent an out-of-bounds error

There is a pre-check of "isInherited" to check if its truly false

The use of require makes makes the code gas-efficient and more readable

modifier onlyBeneficiaryWithIsInherited() {
require(isInherited, "Not inherited yet");
bool isBeneficiary = false;
​
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
​
require(isBeneficiary, "Not a beneficiary");
_;
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.