Christmas Dinner

First Flight #31
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

H-01: Re-Entrancy Vulnerability in Refund Function

Summary

The contract has a re-entrancy vulnerability due to the order of state updates in the nonReentrant modifier. Specifically, the locked flag is set to false after external calls, which allows for potential re-entrancy, enabling attackers to repeatedly call the refund function and exploit the vulnerability.

Vulnerability Details

In the current implementation, the nonReentrant modifier prevents re-entrancy by setting a locked flag to true at the start of the function execution and resetting it to false at the end. However, the issue lies in the timing of the locked state modification:

  1. The locked flag is set to true at the start of the modifier, preventing re-entrancy when the function is first called.

  2. The external calls made within the refund function (e.g., _refundERC20 and _refundETH) might trigger re-entrant calls before the locked flag is reset to false, causing the vulnerability.

This pattern creates an opportunity for an attacker to exploit the contract and perform a re-entrancy attack.

Impact

The impact of this vulnerability is high because it directly opens the door to a re-entrancy attack. If exploited, the attacker could:

  • Drain funds from the contract by repeatedly calling the refund function before the lock is reset.

  • Potentially manipulate the flow of the contract, depending on how other external calls are implemented (e.g., making multiple transfers or withdrawals).

Since the contract is handling financial transactions (such as ETH or ERC20 tokens), the vulnerability could lead to significant monetary losses.

Tools Used

  • Manual code review: The analysis was performed by reviewing the logic of the nonReentrant modifier and the refund function.

Recommendations

To fix the re-entrancy vulnerability and improve the overall safety of the contract, the following changes are recommended:

  • Modify the nonReentrant modifier to set locked = true before the external function calls, and set it to false only after the function execution ends. This will prevent any external calls from re-entering the function while it is in progress.

modifier nonReentrant() {
require(!locked, "No re-entrancy");
+ locked = true; // Locking at the beginning to prevent re-entry
_;
locked = false; // Unlocking after the function body executes
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

mutex lock incomplete

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!