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

Missing ETH transfer in `ChristmasDinner::withdraw` function leads to locked ETH funds

trashpirate

Summary

The ChristmasDinner::withdraw function does not transfer the ETH in the contract to the host. If ETH has been sent to the contract by participants, these funds are fully locked in the contract after the dealine has passed and partipants cannot issue any refunds anymore.

Vulnerability Details

Based on the docs, the contract is supposed to accept ETH for sign ups. However, the ChristmasDinner::withdraw function does not transfer the ETH in the contract to the host. If ETH has been sent to the contract by participants, these funds cannot be withdrawn by the host. Users could still withdraw their funds using the refund function before the deadline but after the deadline the ETH is locked in the contract.

function withdraw() external onlyHost {
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)));
@> // missing line to transfer ETH to host
}

Proof of Concept

The following scenario leads to locked ETH funds:

  1. User signs up for the Christmas dinner and sends ETH to the contract.

  2. The deadline for sign ups passes and the host calls the withdraw function to withdraw the funds.

  3. The host receives the WETH, WBTC and USDC but the ETH remains in the contract.

  4. User cannot withdraw the ETH using the refund function because the deadline has passed.

  5. ETH is locked in contract.

Code:

To demonstrate that the host does not receive deposited ETH funds, place following code into ChristmasDinnerTest.t.sol:

function test_withdrawAsHostIfEthInContract() public {
uint256 ethAmount = 0.5e18;
uint256 wbtcAmount = 2e18;
uint256 usdcAmount = 1e18;
​
// user1 signs up with WBTC
vm.prank(user1);
cd.deposit(address(wbtc), wbtcAmount);
​
// user2 signs up with USDC
vm.prank(user2);
cd.deposit(address(usdc), usdcAmount);
​
// user3 signs up with ETH
deal(user3, ethAmount);
vm.prank(user3);
(bool success,) = address(cd).call{value: ethAmount}("");
require(success, "transfer failed");
​
// host withdraws all funds
uint256 initialFunds = deployer.balance;
vm.prank(deployer);
cd.withdraw();
​
assertEq(wbtc.balanceOf(deployer), wbtcAmount);
assertEq(usdc.balanceOf(deployer), usdcAmount);
assertEq(deployer.balance - initialFunds, ethAmount);
}

Impact

The impact of this issue is high as it prevents access to valuable funds needed for the event, essentially resulting in lost funds as they will be locked in the contract. Since the protocol states that signups with ETH are possible it is also highly likely that users will send ETH to the contract. If the host cannot withdraw these funds, the event might not be able to take place as planned.

Tools Used

Manual review, custom test

Recommendations

To allow the host to withdraw the ETH funds, the ChristmasDinner::withdraw function should transfer the ETH in the contract to the host. This will allow the host to access the funds and use them for the event. Alternatively, implement an additional withdraw function for ETH in the contract.

function withdraw() external onlyHost {
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)));
+ (bool success,) = payable(_host).call{value: address(this).balance}("");
+ require(success, "ETH transfer failed");
}
Updates

Lead Judging Commences

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

withdraw function lacks functionality to send ether

Support

FAQs

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

Give us feedback!