Christmas Dinner

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

Missing Ether Withdrawal in ChristmasDinner.sol Contract

Summary
The withdraw() function in the ChristmasDinner contract does not include logic for withdrawing Ether (ETH). This omission can lead to Ether being stacked indefinitely in the contract, potentially causing financial losses for users and impacting the usability of the contract.

Vulnerability Details
The withdraw() function in the ChristmasDinner contract only handles the transfer of ERC20 tokens back to the host’s address. It does not include any logic for handling Ether deposits. As a result, users who deposit Ether into the contract and later wish to withdraw it cannot do so. This could lead to unexpected accumulation of Ether within the contract, making it inaccessible to users and posing a security risk.

Here is the relevant part of the code that requires modification:

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)));
}

Impact

  • Critical Risk: Users who deposit Ether into the contract and wish to withdraw it are unable to do so. This could lead to financial losses and inconvenience for those users.

  • Inaccessibility of Funds: Ether deposits will remain in the contract indefinitely, causing potential issues such as blocked funds and inability to meet withdrawal requests.

  • Security Concern: Accumulation of Ether without withdrawal mechanisms could lead to vulnerabilities in the contract, impacting its overall security and reliability.

Tools Used

  • Manual Code Review: The vulnerability was identified through a detailed code review of the withdraw() function and its interaction with Ether.

  • Solidity Best Practices Analysis: Reviewing the contract against best practices for handling Ether deposits and withdrawals.

Recommendations
To mitigate the issue, update the withdraw() function to include the ability to transfer Ether from the contract to the host’s address:

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)));
+ payable(_host).call{value: address(this).balance};
}

This modification will ensure that the contract can handle both ERC20 token withdrawals and Ether withdrawals, thus preventing the indefinite accumulation of Ether in the contract and ensuring better user experience and security.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

withdraw function lacks functionality to send ether

0xtimefliez Lead Judge 7 months 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.