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

[C-1]Reentrancy Lock Issue in nonReentrant Modifier

kings5layer

Summary

A critical vulnerability in the nonReentrant modifier fails to prevent reentrancy, allowing attackers to drain funds. By fixing the transient storage key usage and following secure coding practices, contracts can effectively mitigate reentrancy risks.

Vulnerability Details

Affected Component:

  • Modifier: InheritanceManager::nonReentrant()

  • Function: InheritanceManager::sendERC20()

  • Function: InheritanceManager::sendETH()

  • Function: InheritanceManager::contractInteractions()

Root Cause Analysis:

The modifier uses inconsistent transient storage keys, rendering the reentrancy check ineffective.

Faulty Code:

modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) } // ❌ Incorrect key used for checking lock
tstore(0, 1) // ✅ Lock stored at key 0
}
_;
assembly {
tstore(0, 0) // ✅ Unlock stored at key 0
}
}

Problems:

  1. Incorrect Check (tload(1))

    • The function checks key 1, but the lock is stored at key 0.

    • This results in tload(1) always returning 0, failing to detect reentrancy.

  2. No Effective Reentrancy Prevention

    • Since tload(1) == 0 on every execution, attackers can continuously re-enter the function.

Impact

A flaw in the nonReentrant modifier allows reentrancy attacks, which can lead to fund theft, unexpected contract behavior, and potential exploitation of the protocol.

Tools Used

Foundry

Recommendations

✅ Fixing Modifier

Use a consistent key (tload(0)) for checking and setting the lock:

modifier nonReentrant() {
assembly {
if tload(0) { revert(0, 0) } // ✅ Correctly checks reentrancy
tstore(0, 1) // ✅ Locks function
}
_;
assembly {
tstore(0, 0) // ✅ Unlocks function after execution
}
}

✅ Best Practices

  1. Apply the "Checks-Effects-Interactions" Pattern

    • Update state before external calls:

function sendERC20(address _tokenAddress, uint256 _amount, address _to) external nonReentrant onlyOwner {
if (IERC20(_tokenAddress).balanceOf(address(this)) < _amount) {
revert InsufficientBalance();
}
+ _setDeadline();
IERC20(_tokenAddress).safeTransfer(_to, _amount);
- _setDeadline();
}

2 Use OpenZeppelin’s ReentrancyGuard Instead

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract InheritanceManager is ReentrancyGuard {
function sendERC20(address _tokenAddress, uint256 _amount, address _to) external nonReentrant {
// Safe logic
}
}
Updates

Lead Judging Commences

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

Wrong value in nonReentrant modifier

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