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

Vulnerable modifier 'onlyBeneficiaryWithIsInherited' in Inheritancemanager.sol contract (off-by-one error)

simranjitsinghss

Summary:

Hi,

I found out some potential vulnerability in the contract 'Inheritancemanager.sol' in which the modifier 'onlyBeneficiaryWithIsInherited' can leads to off-by-one error.

Vulnerability Details:

The key details of this potential vulnerability can be given as following:

In the modifier 'onlyBeneficiaryWithIsInherited' on line 53-62 of contract 'Inheritancemanager.sol', Loop iterates up to i < beneficiaries.length + 1, causing out-of-bound array access when i == beneficiaries.length, meaning that the last element of the beneficiaries array will be ignored as we are unable to call it via modifier.

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

Impact:

Valid beneficiaries unable to call functions like 'buyOutEstateNFT' and 'appointTrustee' making key inheritance feature unusable.

Tools Used:

Manual Code Analysis

Recommendations:

Implement proper array removal by shifting elements and reducing array length (code given below) or use Openzeppelin's EnumerableSet contract for more robust structure (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/structs/EnumerableSet.sol).

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

Lead Judging Commences

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

Support

FAQs

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