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

Faulty Beneficiary Access Modifier Leading to Array Out-of-Bounds Access

0xbyteknight

Summary

The onlyBeneficiaryWithIsInherited modifier in the InheritanceManager contract uses an incorrect loop condition, causing an array out-of-bounds access. This issue manifests when a beneficiary calls functions protected by this modifier, potentially leading to unintended reverts and denial-of-service.

Vulnerability Details

The modifier is designed to verify that the caller is a beneficiary and that the inheritance flag is set. However, the current implementation uses a loop condition i < beneficiaries.length + 1. When the array has one element, after checking index 0, the loop continues to index 1, which does not exist. This results in a panic due to an array out-of-bounds error. In scenarios where only one beneficiary is present (or if the beneficiary check fails), this flaw prevents legitimate calls, disrupting the contract’s functionality.

Impact

Direct Impact: Legitimate beneficiaries may be unable to execute functions (such as appointing a trustee or triggering fund distribution) because the call reverts due to an out-of-bounds access.

Tools Used

Manual review

Foundry (Forge) for unit testing, which produced an array out-of-bounds panic.

Recommendations

Update the onlyBeneficiaryWithIsInherited modifier to iterate only until i < beneficiaries.length.

Implement an explicit check (using a for loop or mapping) to verify beneficiary membership without risking array over-indexing.

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract InheritanceManagerVulnerabilityTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address beneficiary1 = makeAddr("beneficiary1");
address trustee = makeAddr("trustee");
​
uint256 public constant INITIAL_SUPPLY = 1000e18;
uint256 public constant ASSET_VALUE = 100e18;
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
// Deploy mocks with an initial supply for testing
usdc = new ERC20Mock();
weth = new ERC20Mock();
usdc.mint(owner, INITIAL_SUPPLY);
weth.mint(owner, INITIAL_SUPPLY);
}
​
/// @notice Test that the trustee can set NFT value to zero (potentially affecting buyout calculations).
function test_appointTrusteeFailsWithArrayOutOfBounds() public {
// Arrange
// Setup: add a beneficiary and create an estate NFT.
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.createEstateNFT("Estate", ASSET_VALUE, address(usdc));
vm.stopPrank();
​
// Warp time and trigger inheritance.
vm.warp(3600);
vm.warp(1 + 90 days);
vm.prank(beneficiary1);
im.inherit();
​
// Act & Assert
// Appoint trustee.
vm.prank(beneficiary1);
vm.expectRevert(); // Comment this line to see error.
im.appointTrustee(trustee);
}
}
​
Updates

Lead Judging Commences

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

Support

FAQs

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