Christmas Dinner

Summary

A missing deadline check in the ChristmasDinner::receive function allows users to sign up for the Christmas dinner after the deadline has passed by sending ETH to the contract. This is inconsistent with the deposit restrictions on signups using the whitelisted tokens.

Vulnerability Details

Based on the docs, users can sign up for the Christmas dinner using whitelisted tokens or ETH. The ChristmasDinner::deposit function handling sign ups using whitelisted tokens checks if the deadline has passed before allowing the user to sign up using the beforeDeadline modifier. However, the ChristmasDinner::receive function handling sign ups via ETH does not check the deadline. This allows users to sign up for the event after the deadline has passed by sending ETH to the contract.

Proof of Concept

1. Users sign up for Christmas dinner before deadline

2. Deadline passes

3. User signs up for Christmas dinner after deadline by sending ETH to the contract

4. User's ETH contribution is recorded and the user is considered a participant despite late sign up (assuming receive function is udpated to track partipant's status -> submitted as different issue)

Code:

Place following code into ChristmasDinnerTest.t.sol to demonstrate that ETH deposit is received despite expired deadline:

Impact

This issue is considered as having low impact. No funds are at risk as the ETH is sent to the contract and the user's ETH balance is tracked. However, allowing sign ups after the deadline via ETH transfer defeats the purpose of having a deadline at all. The docs indicate that the desired functionality should prevent deposits and sign ups after the deadline. Thus, the main impact is inconsistency and confusion with the planning of the event if the host is not aware. However, the likelyhood is high that this will cause users knowingly or unknowingly to sign up after the deadline.

Tools Used

Manual review, custom test

Recommendations

There are multiple ways to prevent sign ups after the deadline via ETH transfers:

1. Adding deadline check by adding beforeDeadline modifier to receive function

2. Consider handling ETH sign ups directly in the deposit function instead of using the receive function.

Note: While the second option may change the current contract design, it may be a more consistent approach to handling sign ups. It may be less confusing to users using the same function to sign up with ETH or tokens.