Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Improper Non-Reentrant Guard Implementation

Details:
The nonReentrant modifier is intended to prevent functions from being re-entered, thus protecting against reentrancy attacks. In the contract, the modifier is implemented as follows:

modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}

However, the check is performed using tload(1) while the lock is set with tstore(0, 1). This mismatch means that the guard is checking a different transient storage slot than the one used to set the lock. As a result, the reentrancy lock does not work as intended because it never detects that the function is already executing.

Root Cause:
The root cause is an implementation error where different storage slots are used for reading and writing the reentrancy flag. The check is done on slot 1 (tload(1)) but the flag is set and cleared in slot 0 (tstore(0, 1) and tstore(0, 0)), leading to an ineffective reentrancy guard.

Impact:
Due to the flawed nonReentrant modifier, external functions that rely on this guard (such as those transferring tokens or Ether) may be vulnerable to reentrancy attacks. An attacker could potentially re-enter these functions in the same transaction, leading to unauthorized fund transfers or state manipulations, which can have severe consequences on the contract's integrity and security.

Recommendation:
Correct the nonReentrant implementation by using the same transient storage slot for both checking and setting the guard. For example, the modifier should be implemented as:

modifier nonReentrant() {
assembly {
if tload(0) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}

This change ensures that the lock is correctly set and verified, effectively preventing reentrant calls. Additionally, perform a thorough re-audit and testing (both unit and integration tests) after the change to verify that no further issues exist.

Proof of Concept (PoC):

  1. Setup: Deploy the contract and ensure the nonReentrant modifier is applied to critical functions (e.g., sendERC20, sendETH, and contractInteractions).

  2. Exploit Contract: Create a malicious contract that calls one of these functions and, within the same transaction, triggers a callback (via a fallback function or reentrant call) that re-enters the function.

  3. Observation: Because the reentrancy guard checks tload(1) (which is always false since the lock is stored in slot 0), the malicious contract is able to re-enter the function, thereby executing the external call multiple times.

  4. Result: Multiple executions of the sensitive function allow the attacker to drain funds or cause unexpected state changes.


Updates

Lead Judging Commences

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

Wrong value in nonReentrant modifier

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