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

Array Out-of-Bounds Vulnerability in InheritanceManager.sol

nomadic_bear

Summary

The InheritanceManager contract contains a critical array out-of-bounds vulnerability in the onlyBeneficiaryWithIsInherited modifier. The vulnerability is caused by an incorrect loop termination condition that allows the loop to access memory beyond the array's bounds, resulting in a runtime panic that makes functions inaccessible to non-beneficiary users.

Vulnerability Details

The vulnerability exists in the implementation of the onlyBeneficiaryWithIsInherited modifier:

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length + 1) { // Vulnerability: "+ 1" causes out-of-bounds access
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
_;
}

The issue occurs in the loop condition i < beneficiaries.length + 1, which allows the index i to reach beneficiaries.length. Since arrays in Solidity are 0-indexed, valid indices range from 0 to length-1. When a non-beneficiary calls a function with this modifier, the loop will iterate through all valid indices without finding a match, then attempt to access beneficiaries[beneficiaries.length], which is beyond the array bounds.

PoC

//SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract ArrayOutOfBoundsTest is Test {
InheritanceManager im;
ERC20Mock usdc;
​
address owner = makeAddr("owner");
address beneficiary1 = makeAddr("beneficiary1");
address beneficiary2 = makeAddr("beneficiary2");
address nonBeneficiary = makeAddr("nonBeneficiary");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
}
​
function testArrayOutOfBoundsInOnlyBeneficiaryModifier() public {
// Setup: Add beneficiaries and make inheritance active
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
im.createEstateNFT("test property", 1000, address(usdc));
vm.stopPrank();
​
// Fast forward time to enable inheritance
vm.warp(block.timestamp + 91 days);
// Trigger inheritance
vm.prank(beneficiary1);
im.inherit();
// Verify inheritance is active
assertTrue(im.getIsInherited());
// Try to call a function with the vulnerable modifier from a non-beneficiary
// This should revert with an array out-of-bounds error
vm.startPrank(nonBeneficiary);
// Using expectRevert with a specific panic code for index out-of-bounds (0x32)
vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32));
im.appointTrustee(nonBeneficiary);
vm.stopPrank();
}
function testValidBeneficiaryPassesModifier() public {
// Setup: Add beneficiaries and make inheritance active
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
vm.stopPrank();
​
// Fast forward time to enable inheritance
vm.warp(block.timestamp + 91 days);
// Trigger inheritance
vm.prank(beneficiary1);
im.inherit();
// A valid beneficiary should be able to call functions with the modifier
// Note: This might still revert for other reasons, but not due to the array out-of-bounds bug
vm.startPrank(beneficiary1);
// The actual function call succeeds - not reverting due to array bounds
im.appointTrustee(nonBeneficiary);
// Verify trustee was set correctly
assertEq(im.getTrustee(), nonBeneficiary);
vm.stopPrank();
}
function testBuyOutEstateWithArrayOutOfBounds() public {
// Setup: Add beneficiaries and create NFT
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
im.createEstateNFT("vacation home", 3e6, address(usdc));
vm.stopPrank();
// Fast forward time to enable inheritance
vm.warp(block.timestamp + 91 days);
// Trigger inheritance
vm.prank(beneficiary1);
im.inherit();
// Verify inheritance is active
assertTrue(im.getIsInherited());
// Try to call buyOutEstateNFT from a non-beneficiary address
vm.startPrank(nonBeneficiary);
// This should fail with array out-of-bounds error
vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32));
im.buyOutEstateNFT(1);
vm.stopPrank();
}
function testMultipleBeneficiariesEdgeCase() public {
// Setup: Add several beneficiaries
vm.startPrank(owner);
address[] memory beneficiaries = new address[](5);
for (uint i = 0; i < 5; i++) {
beneficiaries[i] = makeAddr(string(abi.encodePacked("beneficiary", i)));
im.addBeneficiery(beneficiaries[i]);
}
vm.stopPrank();
// Fast forward time to enable inheritance
vm.warp(block.timestamp + 91 days);
// Trigger inheritance
vm.prank(beneficiaries[0]);
im.inherit();
// Verify inheritance is active
assertTrue(im.getIsInherited());
// Try to call a function with the vulnerable modifier from a non-beneficiary
vm.startPrank(nonBeneficiary);
// This should revert with an array out-of-bounds error
vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32));
im.appointTrustee(nonBeneficiary);
vm.stopPrank();
}
}

PoC Result:

forge test --match-contract ArrayOutOfBoundsTest -vvv
[⠰] Compiling...
No files changed, compilation skipped
​
Ran 4 tests for test/ArrayOut-of-BoundsPoC.t.sol:ArrayOutOfBoundsTest
[PASS] testArrayOutOfBoundsInOnlyBeneficiaryModifier() (gas: 294171)
[PASS] testBuyOutEstateWithArrayOutOfBounds() (gas: 294022)
[PASS] testMultipleBeneficiariesEdgeCase() (gas: 227176)
[PASS] testValidBeneficiaryPassesModifier() (gas: 163037)
Suite result: ok. 4 passed; 0 failed; 0 skipped; finished in 5.69ms (1.78ms CPU time)
​
Ran 1 test suite in 40.08ms (5.69ms CPU time): 4 tests passed, 0 failed, 0 skipped (4 total tests)

Impact

  1. Affects Key Inheritance Functions

  2. Denial of Service

  3. Gas Waste: Users who attempt to call these functions (potentially unaware that they are not beneficiaries) will have their transactions revert after consuming gas, without clear indication of why they are not authorized.

Tools Used

Foundry (Forge)

Manual code review

Recommendations

Correct the loop condition:

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length) { // Removed the "+ 1"
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
// Add explicit check after loop
require(i < beneficiaries.length, "Not a beneficiary");
require(isInherited, "Not yet inherited");
_;
}
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.