Christmas Dinner

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

Reentrancy Vulnerability in nonReentrant Modifier

Summary

The nonReentrant modifier in the ChristmasDinner contract is improperly implemented, making it vulnerable to reentrancy attacks. Specifically, the locked = false; statement is executed after the function logic (represented by _) in the modifier. This allows an attacker to exploit the refund function, which relies on this modifier, to repeatedly call the _refundERC20 and _refundETH functions and drain the contract's funds.

The refund function is the most critical function in the contract, as it allows users to withdraw their contributions. By exploiting this vulnerability, an attacker can reenter the refund function and repeatedly call the _refundERC20 and _refundETH functions, withdrawing more than their entitled balance.

Link to code:https://github.com/Cyfrin/2024-12-christmas-dinner/blob/9682dcc306db935a2511e1eb8280d17ef01e9004/src/ChristmasDinner.sol#L77

Vulnerability Details

The nonReentrant modifier sets locked = false after the function body executes, leaving the contract vulnerable to reentrancy attacks during external calls.

  • The refund function, which utilizes this modifier, calls both _refundERC20 and _refundETH. These functions perform external calls (e.g., safeTransfer for ERC20 tokens and transfer for Ether), which can be exploited for reentry.

Affected Functionality:

  • refund()

  • Internal calls to _refundERC20() and _refundETH().

Cause:

The nonReentrant modifier fails to set locked = true before the function body executes. This allows an attacker to reenter the contract while locked remains false.

Impact

An attacker can exploit the reentrancy vulnerability to:

  1. Call the refund function repeatedly before the balance is updated, draining all the ERC20 tokens and Ether from the contract.

  2. Result in significant financial losses for the participants and the host of the event.

Tools Used

Manual code review.

Recommendations

set the locked variable to true:

modifier nonReentrant() {
require(!locked, "No re-entrancy");
locked = true; // that way if anybody tries to re-enter the contract the requirement will fail
_;
locked = false;
}
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!