Inheritable Smart Contract Wallet

Summary

The InheritanceManager::onlyBeneficiaryWithIsInherited modifier uses an unbounded loop that iterates beyond the beneficiaries array bounds, risking out-of-bounds access, transaction reverts, and gas inefficiency. This introduces vulnerabilities in access control logic and exposes the contract to denial-of-service (DoS) attacks via gas exhaustion.

Vulnerability Details

The modifier’s while loop runs beneficiaries.length + 1 times, attempting to access beneficiaries[i] when i = beneficiaries.length. Since Solidity arrays are zero-indexed, this guarantees an out-of-bounds exception, causing the transaction to revert.

Additionally, looping over a dynamic-length array in modifiers is inherently gas-inefficient and risky, as large beneficiaries arrays could cause gas limits to be exceeded.

Impact

Medium Severity

Guaranteed Reverts: Transactions using this modifier will revert due to out-of-bounds access, breaking core functionality.

Access Control Bypass: If the loop is intended to validate msg.sender, legitimate beneficiaries may be denied access.

Gas Exhaustion: Unbounded loops waste gas and could lead to DoS during high network congestion.

Tools Used

Manual code review

Recommendations

Change the loop condition to i < beneficiaries.length to prevent out-of-bounds access:

Split the isInherited check from beneficiary validation to simplify logic:

Replace array iteration with a mapping for efficient beneficiary checks:

These changes eliminate the vulnerability while improving code readability and gas efficiency.