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

Missing update to participant status in the `ChristmasDinner::receive` function fails to track user sign ups via ETH transfers

trashpirate

Summary

The ChristmasDinner::receive function does not update the participant status when a user signs up via an ETH transfer. This can lead to inconsistencies in the participant status and prevent the host from tracking the number of participants that signed up via ETH.

Vulnerability Details

Based on the docs, users can sign up for the Christmas dinner by sending ETH to the contract. The ChristmasDinner::receive function is used to receive the ETH and update the participant status. However, the implementation of the receive function does not set the partipant's status in the participants mapping to true. This means that the host does not know who signed up via ETH unless they review the transaction history manually.

receive() external payable {
@> // missing line to set participant status to true
etherBalance[msg.sender] += msg.value;
emit NewSignup(msg.sender, msg.value, true);
}

Proof of Concept

The following scenario can lead to inconsistencies in the participant status:

  1. User signs up for the Christmas dinner by sending ETH to the contract.

  2. The receive function is called when the ETH is received but the participant status is not updated.

  3. The user's partipant status is still set to false.

Code:

To demonstrate the failing of ETH signups place following code into ChristmasDinnerTest.t.sol:

function test_depositBeforeDeadlineWithEth() public {
address payable _cd = payable(address(cd));
​
// user1 signs up with ETH
deal(user1, 10e18);
vm.prank(user1);
(bool sent,) = _cd.call{value: 1e18}("");
require(sent, "transfer failed");
​
// check user1 partipant status
assertEq(cd.getParticipationStatus(user1), true);
}

Impact

The impact of this issue is medium as it can significantly affect the functionality of the protocol but no funds are at risk. The main purpose of the protocol is to track participant signups and collect funds for the event. If the protocol consistently fails to consider partipants that sign up via ETH, the host will not have the correct number of partipants for the event and may not be able to plan accordingly. Since the protocol states that signups with ETH are possible it is also highly likely that some users will use this option.

Tools Used

Manual review, custom test

Recommendations

Update the ChristmasDinner::receive function to set the participant status to true when a user signs up via an ETH transfer. This will ensure that the host can track all participants that signed up for the event and plan accordingly. Alternatively, ETH transfers could be treated solely as generous additional contributions which should be indicated as such through a GenerousAdditionalContribution event instead of a NewSignup event.

receive() external payable {
+ participant[msg.sender] = true;
etherBalance[msg.sender] += msg.value;
emit NewSignup(msg.sender, msg.value, true);
}
Updates

Lead Judging Commences

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

receive does not update participation status

Support

FAQs

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

Give us feedback!