Inheritable Smart Contract Wallet

Summary

The security issue in the onlyBeneficiaryWithIsInherited modifier represents a critical access control vulnerability. By using the condition i < beneficiaries.length + 1, the loop allows index i to reach beneficiaries.length, which exceeds array bounds. In Solidity, accessing an out-of-bounds storage array index returns address(0) rather than reverting. Consequently, if msg.sender is address(0), it could pass the access check when i == beneficiaries.length. This vulnerability would allow unauthorized access to restricted functions like withdrawInheritedFunds and buyOutEstateNFT, potentially resulting in direct financial loss. The severity of this issue is HIGH due to its potential impact on fund security and the fundamental nature of this access control mechanism

Vulnerability Details

Modifier Access Control Bypass via Out-of-Bounds Array Access

Description

The onlyBeneficiaryWithIsInherited modifier contains a critical flaw in its array access verification logic. The condition i < beneficiaries.length + 1 in the while loop permits the index variable i to reach the value of beneficiaries.length, which is outside the valid array bounds (valid indices are 0 to length-1).

Technical Impact

When i equals beneficiaries.length, the code attempts to access beneficiaries[i], which is out of bounds. In Solidity, reading from an out-of-bounds index in a storage array returns the default value for that type (address(0) for an address array) instead of reverting.

If msg.sender is address(0) and isInherited is true, the condition msg.sender == beneficiaries[i] && isInherited will evaluate to true during this out-of-bounds access, allowing unauthorized execution of protected functions.

Vulnerable Functions

Functions using this modifier that are at risk:

• withdrawInheritedFunds(address)

• buyOutEstateNFT(uint256)

• appointTrustee(address)

Attack Vector

An attacker with control of address(0) could call restricted functions after the inheritance has been triggered (isInherited is true), potentially:

1. Withdrawing all inherited funds

2. Manipulating estate NFTs

3. Replacing the appointed trustee

Code Context

The vulnerability exists in the context of an inheritance management contract where controlled access to funds distribution is essential for maintaining the intended inheritance logic.

Tools Used

manual review

Recommendations

Replace the vulnerable while loop with a properly bounded for loop that ensures array access remains within bounds:

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