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

Reentrancy vulnerability in `ChristmasDinner::refund()`

owanemi

Summary

The refund() fucntion is vulnerable to Re-entrancy attacks by a malicious user. The vulnerability is caused by an incorrect implementation of the nonReentrant modifier and failure to follow the Checks-Effects-Interactions pattern.

Vulnerability Details

The nonReentrant modifier is implemented incorrectly and so state changes will occur after external calls, allowing for reentrancy. Specifically:

  1. The modifier never sets locked = true, only checks and sets it to false:

modifier nonReentrant() {
require(!locked, "No re-entrancy");
_; // Function executes with lock still false
locked = false; // Lock is set to false after execution
}

2.The refund() function performs external calls before state changes:

function refund() external nonReentrant beforeDeadline {
address payable _to = payable(msg.sender);
_refundERC20(_to); // External calls happen first
_refundETH(_to); // External calls happen first
emit Refunded(msg.sender);
}

3.The _refundETH() function makes external calls before updating state:

function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
_to.transfer(refundValue); // External call happens before state update
etherBalance[_to] = 0; // State is updated too late
}

This allows a malicious contract to:

  1. Make an initial deposit to become a participant

  2. Call refund()

  3. When receiving ETH in the fallback function, call refund() again

  4. Repeat step 3 until the contract is drained

Impact

A malicious user/contract can drain the entire ETH balance from our contract. This is a critical severity issue because

  1. It allows unauthorized withdrawal of funds

  2. Can drain 100% of the contract's ETH balance

  3. Requires minimal upfront capital to execute

  4. Can be executed by any participant

Tools Used

Slither, Remix IDE

Recommendations

  1. Follow CEI pattern (do necessary checks first, change the state, then do the interaction)

  2. Use Openzeppelin's Reentrancy Guard contract

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!