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

Users can register as participants even without sending any whitelisted ERC20 token or ETH

marcoroccon

Description:
The function changeParticipationStatus can be called by anyone, since there are neither controls nor modifiers that prevent random callers to be listed as participants.

Impact:
This problem is considered to be high-severity, since this will heaviliy interfere with the organization of the Christmas dinner. Indeed, a random user could trick the host and list themselves as a participant even without sending any whitelisted ERC20 token or ETH.

In the documentation for ChristmasDinner it is clearly stated that
"with our Christmas Dinner Contract we directly "force" the attendees to pay upon signup".

The function changeParticipationStatus allows, in fact, the prohibited behavior, since there is no enforcement of payment for users to be listed as participants.

Tools Used:
Manual review

Proof of concept:

Add this test to ChristmasDinnerTest.t.sol. This demonstrates that a user can change their participation status to true even though they didn't deposit any whitelisted ERC20 token or ETH.

function test_changeParticipationStatusWithoutDeposit() public {
vm.startPrank(user1);
cd.changeParticipationStatus();
assertEq(cd.getParticipationStatus(user1), true);
}

Recommended mitigation
The proposed mitigation aims at maintaining a strong readbility of code. To achieve this it is recommended to add a private function hasDepositedFunds to check whether the user has already deposited one (o more) of the whitelisted ERC20 token or ETH. A new modifier is then created to implement this control on the changeParticipationStatus function. In case the user has not deposited any funds, the modifier will cause the function to revert with a custom error OnlyUserWithDepositedFundsCanChangeTheirStatus.

error NotHost();
error BeyondDeadline();
error DeadlineAlreadySet();
error OnlyParticipantsCanBeHost();
error NotSupportedToken();
+ error OnlyUserWithDepositedFundsCanChangeTheirStatus();
+ modifier onlyUsersWithDepositedFunds() {
+ if (hasDepositedFunds(msg.sender)) {
+ _;
+ } else {
+ revert OnlyUserWithDepositedFundsCanChangeTheirStatus();
+ }
+ }
​
​
+ function hasDepositedFunds(address user) private 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 onlyUsersWithDepositedFunds {
if(participant[msg.sender]) {
participant[msg.sender] = false;
} else if(!participant[msg.sender] && block.timestamp <= deadline) {
participant[msg.sender] = true;
} else {
revert BeyondDeadline();
}
emit ChangedParticipation(msg.sender, participant[msg.sender]);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge about 1 year 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.

Give us feedback!