Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Invalid

Funds Not Transferred to Owner in inherit() with Single Beneficiary

Summary

The inherit function, when called with a single beneficiary after 90 days of inactivity, is documented to allow the owner to "reclaim this contract" (implying funds). However, it only reassigns the owner variable to msg.sender without transferring any funds. This leaves the contract’s assets (ETH and tokens) inaccessible to the caller, contradicting the expected behavior of reclaiming funds.

Vulnerability Details

The vulnerable code is within the inherit function:

solidity

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
//@audit-issue k 'After 90 Days the owner cannot reclaim his funds via `InheritanceManager::inherit()` it only transfer the ownership to the caller`
owner = msg.sender;
_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 test_inheritOnlyOneBeneficiaryDoesntGetFunds() public {
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 10e10);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
vm.stopPrank();
assertEq(user1, im.getOwner());
assertEq(address(user1).balance, 0);
console.log(address(user1).balance);
}
}

No Fund Transfer:

  • The comment suggests the owner can "reclaim this contract from beneficiaries slot0," which implies regaining control over its funds.

  • However, the function only updates owner = msg.sender and resets the deadline, leaving ETH and ERC20 tokens in the contract’s balance.

  • Misleading Intent:

    • Users expect "reclaiming" to include asset transfer, but the owner must separately call functions like sendETH or sendERC20 to access funds, which isn’t automatic or implied by inherit().

Impact

Funds Inaccessibility: The owner regains control but cannot immediately access funds, requiring additional transactions that may be overlooked or impossible if keys are truly lost.

  • User Confusion: The mismatch between documentation and behavior misleads users expecting automatic fund recovery.

  • Limited Severity: While not a direct security exploit, it reduces the contract’s usability and could lock funds if the owner doesn’t manually withdraw them post-reclaim.

Tools Used

Manual Review

Recommendations

Modify inherit to transfer funds to the owner in the single-beneficiary case:

solidity

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
if (msg.sender != owner) {
revert NotOwner(msg.sender);
}
owner = msg.sender; // Optional, as it’s already the owner
uint256 ethBalance = address(this).balance;
if (ethBalance > 0) {
(bool success, ) = msg.sender.call{value: ethBalance}("");
require(success, "ETH transfer failed");
}
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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