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

H-4: Lack of Ether Withdrawal Mechanism in withdraw Function

andrekkr

Summary

The withdraw function in the contract allows the host to withdraw ERC20 tokens from the contract, but it fails to provide a way for Ether (ETH) sent to the contract to be withdrawn. This leaves any ETH deposited in the contract "locked", as there is no handling for it in the original function. The proposed fix adds the ability for the host to withdraw ETH using the .call method.

Vulnerability Details

Root Cause: The original withdraw function only supports the withdrawal of ERC20 tokens (WETH, WBTC, USDC) and does not include a mechanism to withdraw Ether (ETH) sent to the contract. As a result, any Ether sent to the contract is stuck in the contract and cannot be withdrawn.

  • Expected Behavior: The contract should allow the host to withdraw both ERC20 tokens and Ether (ETH) sent to the contract.

  • Current Behavior: The contract only handles the withdrawal of ERC20 tokens. Ether (ETH) sent to the contract is "trapped" and cannot be retrieved by the host.

Impact

The failure to allow withdrawal of Ether sent to the contract results in funds being locked in the contract. If the contract is used in a way that Ether is deposited into it, the absence of this feature could create financial risks or prevent proper management of funds.

Tools Used

  • Manual code review.

  • Foundry for testing

Recommendations

1 - Implement Ether Withdrawal: Modify the withdraw function to support the withdrawal of Ether. This can be done using the .call method, which is a more secure way to transfer Ether compared to .transfer due to its gas limitations.

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)));
// Allow host to withdraw ETH
(bool success, ) = _host.call{value: address(this).balance}("");
if (!success) {
revert EtherTransferFailed();
}
}

  • Security Considerations: Ensure the use of .call{value: amount}("") to handle Ether transfers, as it avoids gas stipend limitations present in .transfer and gives the receiver more flexibility. Adding a revert() with a message on failure ensures that the failure reason is clear.

PoC

function testWithdrawEther() public {
vm.deal(address(this), 10 ether);
assertEq(address(this).balance, 10 ether);
vm.prank(deployer);
cd.withdraw();
assertEq(address(host).balance, 10 ether);
}
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!