Christmas Dinner

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

Mismatch between deposit functionality and documentation, preventing signing up other users

Summary

The deposit function in the ChristmasDinner contract does not allow a user to sign up other users despite the documentation stating otherwise. This mismatch between the intended behavior and the actual implementation could lead to confusion at the participiants.

Vulnerability Details

The deposit function is supposed to allow users to sign up other participants on their behalf by depositing tokens for them. However, the current implementation checks the participant mapping against msg.sender, which ties the logic strictly to the caller of the function.

if(participant[msg.sender]){
balances[msg.sender][_token] += _amount;
IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);
emit GenerousAdditionalContribution(msg.sender, _amount);
} else {
participant[msg.sender] = true;
balances[msg.sender][_token] += _amount;
IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);
emit NewSignup(msg.sender, _amount, getParticipationStatus(msg.sender));
}

There is no option to specify an address other than msg.sender, meaning a user cannot sign up another participant, contradicting the documentation.

Impact

This issue results in a functional limitation of the deposit function. While it does not directly lead to financial loss or exploits, it creates confusion due to the mismatch between the intended functionality described in the documentation and the actual behavior. This could cause trust issues among users and disrupt the contract's usability.

Tools Used

Manual code review

Recommendations

To allow users to sign up other participants, the deposit function should be modified to accept an additional parameter, such as _user, which specifies the address to be signed up.

function deposit(address _user, address _token, uint256 _amount) external beforeDeadline {
if(!whitelisted[_token]) {
revert NotSupportedToken();
}
if(participant[_user]){
balances[_user][_token] += _amount;
IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);
emit GenerousAdditionalContribution(_user, _amount);
} else {
participant[_user] = true;
balances[_user][_token] += _amount;
IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);
emit NewSignup(_user, _amount, getParticipationStatus(_user));
}
}
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!