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

Allowing host to withdraw funds before deadline has expired may prevent users from receiving refunds

trashpirate

Summary

There are no time restrictions on the ChristamsDinner::withdraw function which allows the host to withdraw funds before the deadline has expired. If the host withdraws the funds before the deadline has passed, users will not be able to receive refunds for their deposits.

Vulnerability Details

Based on the specs, the ChristamsDinner::refund function should allow users to issue a refund of their deposits if they decide that they don't want to attend the event anymore. The ChristamsDinner::withdraw function is intended to allow the host to withdraw the funds after the deadline has passed. However, there are no time restrictions on the withdraw function which allows the host to withdraw the funds before the deadline has expired. If the host withdraws the funds before the deadline has passed, users will not be able to receive refunds for their deposits.

function withdraw() external onlyHost {
@> // missing deadline check
address _host = getHost();
i_WETH.safeTransfer(_host, i_WETH.balanceOf(address(this)));
i_WBTC.safeTransfer(_host, i_WBTC.balanceOf(address(this)));
i_USDC.safeTransfer(_host, i_USDC.balanceOf(address(this)));
}

Proof of Concept

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

  2. Host does not check the exact deadline and withdraws the funds a day early.

  3. User can't attend the event anymore and wants to issue a refund.

  4. User calls the refund function but the funds are not available in the contract anymore. The function reverts.

Code:

Place following code into CrhistmasDinnerTest.t.sol:

function test_revert_refundWithinDeadline() public {
uint256 depositAmount = 1e18;
​
// user1 signs up with WETH
vm.prank(user1);
cd.deposit(address(weth), depositAmount);
assertEq(weth.balanceOf(address(cd)), depositAmount);
​
// time passes but host withdraws 1 day early
vm.warp(6 days);
address host = cd.getHost();
vm.prank(host);
cd.withdraw();
​
// user decides last minute but within deadline to issue refund
vm.expectRevert();
vm.prank(user1);
cd.refund();
}

Impact

The impact of this issue is low as no funds are lost. However, it can lead to user dissatisfaction if they are unable to receive refunds for their deposits. If hosts always withdraw funds after deadline, this is not an issue. But mistakes can happen and they are easily avoided by adding a deadline check.

Tools Used

Foundry, manual review, custom test

Recommendations

To avoid mistakes by the host and withdraw funds early, it is recommended to add a deadline check in the ChristmasDinner::withdraw function. This will ensure that the host can only withdraw the funds after the deadline has passed and users have had the opportunity to issue refunds if needed.

function withdraw() external onlyHost {
+ require(block.timestamp > deadline, "Deadline has not passed yet");
address _host = getHost();
i_WETH.safeTransfer(_host, i_WETH.balanceOf(address(this)));
i_WBTC.safeTransfer(_host, i_WBTC.balanceOf(address(this)));
i_USDC.safeTransfer(_host, i_USDC.balanceOf(address(this)));
}
Updates

Lead Judging Commences

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

withdraw is callable before deadline ends

Support

FAQs

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

Give us feedback!