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

Anyone can sign up for the dinner without funding it

silverologist

Summary

There are multiple ways to sign up for the dinner without actually funding it:

  • ChristmasDinner::changeParticipationStatus allows signing up for the dinner without funding it.

  • Funding amount is not checked to be non-zero in ChristmasDinner::deposit() nor in ChristmasDinner:receive() either.

  • Anyone can sign up for the dinner by funding it, then get a refund and still be considered a participant.

Although no mimum funding amount is implemented, allowing participants to sign up with amount 0 will effectively defeat the purpose of the contract, whis is defined in its About section: with our Christmas Dinner Contract we directly "force" the attendees to pay upon signup, so the host can plan properly knowing the total budget after deadline.

Vulnerability Details

When someone who is not a participant of the contract calls ChristmasDinner:changeParticipationStatus before the deadline, they will be set as a participant despite bypassing the depositing of funds:

function changeParticipationStatus() external {
if (participant[msg.sender]) {
participant[msg.sender] = false;
} else if (!participant[msg.sender] && block.timestamp <= deadline) {
// @audit -- this branch will be executed
participant[msg.sender] = true;
} else {
revert BeyondDeadline();
}
emit ChangedParticipation(msg.sender, participant[msg.sender]);
}

Currently there is no minimum funding amount in order to sign up using ChristmasDinner:deposit() or ChristmasDinner:receive() either, which again allows participants to sign up without funding the dinner at all, defeating the purpose of the contract.

The ChrismasDinner:refund() function should also reset the participation status of a user in order to prevent them from showing up to the dinner after being refunded.

Impact

Anyone can sign up without funding the dinner.

Tools Used

Recommendations

Add the following lines:

+ function hasContributed(address _user) internal view returns (bool) {
+ return balances[_user][address(i_WETH)] > 0 ||
+ balances[_user][address(i_WBTC)] > 0 ||
+ balances[_user][address(i_USDC)] > 0 ||
+ etherBalance[_user] > 0;
+ }
​
function changeParticipationStatus() external {
if (participant[msg.sender]) {
participant[msg.sender] = false;
- } else if (!participant[msg.sender] && block.timestamp <= deadline) {
+ } else if (!participant[msg.sender] && block.timestamp <= deadline && hasContributed(msg.sender)) {
participant[msg.sender] = true;
} else {
revert BeyondDeadline();
}
emit ChangedParticipation(msg.sender, participant[msg.sender]);
}
function deposit(address _token, uint256 _amount) external beforeDeadline {
+ require(_amount > 0);
if (!whitelisted[_token]) {
receive() external payable {
+ require(msg.value > 0);
etherBalance[msg.sender] += msg.value;
function refund() external nonReentrant beforeDeadline {
address payable _to = payable(msg.sender);
+ participants[msg.sender] = false;
_refundERC20(_to);
_refundETH(_to);
emit Refunded(msg.sender);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

usage of change participation logic circumvents deposit

Support

FAQs

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