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

Zero deposit allowed

k3ysk1lls

Summary

Failure to check _amount in the ChristmasDinner::deposit() function means that anyone can sign up for dinner without paying for it.

Vulnerability Details & Impact

Code reference in repo: see code

As it says in the documentation:

  1. we directly "force" the attendees to pay upon signup, so the host can plan properly knowing the total budget after deadline.

  2. (known issues) We are aware that we do not require a minimum deposit amount to sign up as participant for this contract. We consider it not necessary and rely here on social conventions.

I understand that the developer assumed that everyone who wanted to go out to dinner would pay a certain amount. What is not taken into account is that you can pay zero to become a participant. Unfortunately, not checking an amount to pay means you can pay zero and be invited to dinner.

Additionally, people who have already registered and contributed 0 again are considered to have contributed Generius Donation , which is highlighted by the emission of the ChristmasDinner::GenerousAdditionalContribution(address indexed, uint256 indexed) event.

Tools Used

See function testCanDepositZero() in the foundry test below:

//SPDX-License-Identifier: MIT
​
pragma solidity 0.8.27;
​
import {Test, console} from "forge-std/Test.sol";
import {ChristmasDinner} from "../src/ChristmasDinner.sol";
import {ERC20Mock} from "../lib/openzeppelin-contracts/contracts/mocks/token/ERC20Mock.sol";
​
contract XmasDinnerTest is Test {
ChristmasDinner cd;
ERC20Mock wbtc;
ERC20Mock weth;
ERC20Mock usdc;
uint256 constant DEADLINE = 7;
address deployer = makeAddr("deployer");
address user1;
​
function setUp() public {
wbtc = new ERC20Mock();
weth = new ERC20Mock();
usdc = new ERC20Mock();
vm.startPrank(deployer);
cd = new ChristmasDinner(address(wbtc), address(weth), address(usdc));
vm.warp(1);
cd.setDeadline(DEADLINE);
vm.stopPrank();
_makeParticipants();
}
​
function testCanDepositZero() public {
//assert user1 is not yet a participant
assert(!cd.getParticipationStatus(user1));
vm.startPrank(user1);
cd.deposit(address(usdc), 0);
//assert user1 is a participant
assert(cd.getParticipationStatus(user1));
​
//additional generous donation of 0 USDC
vm.expectEmit();
emit ChristmasDinner.GenerousAdditionalContribution(user1, 0);
cd.deposit(address(usdc), 0);
vm.stopPrank();
}
function _makeParticipants() internal {
user1 = makeAddr("user1");
wbtc.mint(user1, 2e18);
weth.mint(user1, 2e18);
usdc.mint(user1, 2e18);
vm.startPrank(user1);
wbtc.approve(address(cd), 2e18);
weth.approve(address(cd), 2e18);
usdc.approve(address(cd), 2e18);
vm.stopPrank();
}
​
​
}

Recommendations

I think the easiest way is to set constant for each accepting token and ETH that represent minimum amount that can be deposited. This should then be tested in the deposit() and receive() functions.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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