The onlyBeneficiaryWithIsInherited modifier in the Solidity contract contains a potential out-of-bounds array access issue due to an off-by-one error in the loop condition. This can cause transactions to revert unexpectedly, leading to denial of service (DoS) vulnerabilities and preventing legitimate beneficiaries from executing functions that depend on this modifier.
The modifier iterates over the beneficiaries array to check if msg.sender is a valid beneficiary and if isInherited is true. However, the loop condition is written as:
Since i iterates from 0 to beneficiaries.length + 1, there is a scenario where i reaches beneficiaries.length, leading to an attempt to access beneficiaries[beneficiaries.length], which is out of bounds. In Solidity, this results in a runtime exception, reverting the transaction.
Denial of Service (DoS): If an out-of-bounds error occurs, transactions calling functions that rely on this modifier will always revert, blocking access to critical contract functionality.
Invalid State Handling: The contract may fail to execute legitimate operations, preventing rightful beneficiaries from claiming their inheritance.
Unexpected Contract Behavior: The off-by-one logic flaw introduces inconsistencies in execution, which could lead to unpredictable behavior if not properly handled.
Manual Code Review
Fix the Loop Condition: Change the loop condition to iterate correctly within valid bounds:
Implement Boundary Checks: Before accessing an array index, ensure that i is within the allowed range.
Add Unit Tests: Create test cases to validate boundary conditions, including scenarios with an empty beneficiaries array.
Consider Using for Loop: A for loop with an explicit boundary check may improve readability and safety.
By implementing these fixes, the contract will avoid unexpected failures and ensure legitimate beneficiaries can access their inheritance without disruption.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.