Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

Incorrect Placement of isInherited Check

Summary

This report details a critical logical vulnerability identified within InheritanceManager.sol's onlyBeneficiaryWithIsInherited modifier. Specifically, the vulnerability lies in the misplaced isInherited check, which occurs after an array access operation. This design flaw creates a scenario where an out-of-bounds array read is attempted regardless of the isInherited status

Vulnerability Details

The onlyBeneficiaryWithIsInherited modifier intends to restrict function execution to beneficiaries who possess a specific inherited status, represented by the boolean variable isInherited. However, the implementation prioritizes array access over the isInherited check, resulting in a logical inconsistency.

Premature Array Access: The core issue is that the code attempts to access beneficiaries[i] before verifying the isInherited condition. This means that regardless of whether the caller has the required inherited status, the array access will proceed.

Logical Inconsistency: The intent of the modifier is to ensure that only beneficiaries with inherited status can execute the protected function. However, the current implementation violates this intent by attempting array access regardless of the isInherited value.

Impact

Increased Gas Consumption: The unnecessary array access and loop iterations contribute to higher gas costs for transactions, even when the isInherited condition is not met.

Tools Used

Manual Review

Recommendations

Prioritize isInherited Check: The isInherited check should be performed before any array access. This ensures that the array is accessed only when the caller has the required inherited status.

  • Early Return or Revert: If isInherited is false, the modifier should immediately return or revert, preventing unnecessary computations and array access.

modifier onlyBeneficiaryWithIsInherited() {
require(isInherited, "Not inherited"); // Check isInherited FIRST
bool found = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
found = true;
break;
}
}
require(found, "Not a beneficiary");
_;
}
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.