Summary
ChristmasDinner::withdraw() does not withdraw the Ether and the Ether will be stuck in the contract forever after deadline has passed. As ChristmasDinner::refund() will revert as deadline has passed
Vulnerability Details
ChristmasDinner::withdraw() only withdraws the ERC20 tokens and not including Ether. Hence, the ether will be stucked in the contract after deadline.
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
User1 deposit the ERC20 tokens and Ether to ChristmasDinner contract
Deployer withdraws and only the ERC20 tokens are withdrawn. Leaving the Ether inside the contract
Fast forward past deadline, the Ether will be stucked as ChristmasDinner::withdraw() does not withdraw Ether and ChristmasDinner::refund() will revert when users call
function testWithdrawDoesNotIncludeEther() public {
uint256 amount = 2e18;
vm.deal(user1, 5 ether);
vm.startPrank(user1);
cd.deposit(address(wbtc), amount);
cd.deposit(address(weth), amount);
cd.deposit(address(usdc), amount);
(bool success1, ) = address(cd).call{value: 5 ether}("");
require(success1, "Deposit ether failed for user 1");
vm.stopPrank();
console.log("Before withdraw()");
console.log("CD WBTC: ", wbtc.balanceOf(address(cd)));
console.log("CD WETH: ", weth.balanceOf(address(cd)));
console.log("CD USDC: ", usdc.balanceOf(address(cd)));
console.log("CD ETH: ", address(cd).balance);
vm.startPrank(deployer);
cd.withdraw();
console.log("\nAfter withdraw()");
console.log("CD WBTC: ", wbtc.balanceOf(address(cd)));
console.log("CD WETH: ", weth.balanceOf(address(cd)));
console.log("CD USDC: ", usdc.balanceOf(address(cd)));
console.log("CD ETH: ", address(cd).balance);
assertEq(wbtc.balanceOf(address(cd)), 0, "WBTC not empty.");
assertEq(weth.balanceOf(address(cd)), 0, "WETH not empty.");
assertEq(usdc.balanceOf(address(cd)), 0, "USDC not empty.");
assertEq(address(cd).balance, 5 ether, "Ether is empty");
vm.stopPrank();
vm.warp(10 days);
vm.startPrank(user1);
vm.expectRevert(ChristmasDinner.BeyondDeadline.selector);
cd.refund();
vm.stopPrank();
}
Results
[PASS] testWithdrawDoesNotIncludeEther() (gas: 304700)
Logs:
Before withdraw()
CD WBTC: 2000000000000000000
CD WETH: 2000000000000000000
CD USDC: 2000000000000000000
CD ETH: 5000000000000000000
After withdraw()
CD WBTC: 0
CD WETH: 0
CD USDC: 0
CD ETH: 5000000000000000000
Traces:
....
....
....
├─ [0] VM::warp(864000 [8.64e5])
│ └─ ← [Return]
├─ [0] VM::startPrank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [0] VM::expectRevert(BeyondDeadline())
│ └─ ← [Return]
├─ [2514] ChristmasDinner::refund()
-> │ └─ ← [Revert] BeyondDeadline()
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.85s (246.21ms CPU time)
Tools Used
Foundry
Recommendations
Include withdrawing Ether from 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)));
+ uint256 balance = address(this).balance;
+ (bool success, ) = address(_host).call{value: balance}("");
+ require(success);
}
