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

Single beneficiary inherit() does not set isInherited flag, blocking critical functionality

0xWithdrawers

Description

In the InheritanceManager contract, there is a critical logic flaw in the inherit() function that creates inconsistent behavior when there is only one beneficiary. When inherit() is called with a single beneficiary, the function transfers ownership to the beneficiary but does not set the isInherited flag to true.

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

This creates a paradoxical situation where the beneficiary has successfully inherited ownership of the contract but cannot access functions protected by the onlyBeneficiaryWithIsInherited modifier. This is because this modifier requires the isInherited flag to be true, which never happens in the single-beneficiary scenario:

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length + 1) {
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
_;
}

Impact

  1. Functional Inconsistency: A single beneficiary who has successfully claimed ownership through inherit() has fewer privileges than multiple beneficiaries who have done the same, despite having greater ownership rights.

  2. Critical Function Lockout: The sole beneficiary cannot access important contract functions such as buyOutEstateNFT() and appointTrustee(), which are protected by the onlyBeneficiaryWithIsInherited modifier.

Proof of Concept (PoC)

The following code demonstrates how a single beneficiary cannot access functions protected by the onlyBeneficiaryWithIsInherited modifier after inheriting ownership:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "../src/InheritanceManager.sol";
​
// @audit-note - [VULN 08]
// This PoC demonstrates that when there is a single beneficiary,
// the inherit() function does not set isInherited to true,
// causing functions with onlyBeneficiaryWithIsInherited modifier to fail.
​
contract PoCTest is Test {
InheritanceManager inheritanceManager;
address beneficiary = address(0x1);
function setUp() public {
// Deploy the InheritanceManager contract
inheritanceManager = new InheritanceManager();
// Add a single beneficiary
vm.prank(inheritanceManager.getOwner());
inheritanceManager.addBeneficiery(beneficiary);
}
function test_InheritWithSingleBeneficiaryDoesNotSetIsInherited() public {
// Fast forward time to pass the TIMELOCK period (90 days)
vm.warp(block.timestamp + 91 days);
// Get initial value of isInherited
bool initialIsInherited = inheritanceManager.getIsInherited();
console.log("Initial value of 'inheritanceManager.getIsInherited()':", initialIsInherited);
// Call inherit as the beneficiary
vm.prank(beneficiary);
inheritanceManager.inherit();
// Check if msg.sender became the owner
address newOwner = inheritanceManager.getOwner();
console.log("New owner after inherit():", newOwner);
console.log("Beneficiary address:", beneficiary);
assertEq(newOwner, beneficiary, "Beneficiary should become the new owner");
// Check if isInherited is still false
bool isInheritedAfter = inheritanceManager.getIsInherited();
console.log("Value of 'isInherited' after inherit():", isInheritedAfter);
assertEq(isInheritedAfter, false, "isInherited should still be false");
// Now try to call a function protected by onlyBeneficiaryWithIsInherited
console.log("\nAttempting to call appointTrustee() with onlyBeneficiaryWithIsInherited modifier:");
vm.prank(beneficiary);
// This should revert with array out of bounds error, even though the caller is the owner and a beneficiary
vm.expectRevert();
inheritanceManager.appointTrustee(address(0x3));
console.log("Call to 'inheritanceManager.appointTrustee' has reverted");
}
}

Place the test in the test folder and run it with the following command:

forge test --match-test test_InheritWithSingleBeneficiaryDoesNotSetIsInherited -vv

Recommendation

There are two possible approaches to fix this vulnerability, depending on the intended contract behavior:

  1. Set isInherited to true in all valid inheritance scenarios:

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender;
isInherited = true; // Set isInherited to true here as well
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

  1. Alternatively, modify the onlyBeneficiaryWithIsInherited modifier to also allow the owner to execute the functions:

modifier onlyBeneficiaryWithIsInherited() {
if (msg.sender == owner) {
// Allow owner to bypass the check
_;
return;
}
bool isAuthorized = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i] && isInherited) {
isAuthorized = true;
break;
}
}
require(isAuthorized, "Not a beneficiary or inheritance not active");
_;
}

Concrete Impact Example

Consider this scenario:

  • A parent sets up an inheritance contract with their only child as the sole beneficiary

  • The parent becomes incapacitated, and after the 90-day inactivity period, the child calls inherit() to claim ownership

  • The child wants to buy out an NFT representing a family property, which requires calling buyOutEstateNFT()

  • Despite being both the owner and sole beneficiary, the child cannot execute this function because isInherited was never set to true

  • The assets represented by NFTs become permanently locked in the contract, despite the successful transfer of ownership

This scenario demonstrates how a single beneficiary faces a worse outcome than multiple beneficiaries would in the same situation, creating an unexpected and potentially devastating outcome for the intended inheritance plan.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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