Christmas Dinner

Summary

The nonReentrant modifier in the ChristmasDinner contract fails to properly implement reentrancy protection because it never sets the locked state to true, allowing potential reentrant calls.

Vulnerability Details

The modifier checks if locked is false but never sets it to true before executing the function body. This means that during the execution of the protected function, the locked state remains false, allowing another call to enter the same function.

A proper reentrancy guard should:

1. Check if the contract is locked

2. Set the lock before executing the function

3. Release the lock after execution

Impact

High severity. This vulnerability could allow malicious actors to perform reentrant calls on functions meant to be protected, particularly the refund() function which handles both ERC20 and ETH transfers. This could potentially lead to:

• Multiple refunds being processed simultaneously

• Draining of contract funds beyond user entitlements

• Race conditions in state updates

Tools Used

Manual review

Recommendations

Modify the nonReentrant modifier to properly implement the locking mechanism

Consider using OpenZeppelin's ReentrancyGuard contract instead of implementing a custom solution: