ETH stuck in contract
Description
-
The game has a receive() external payable {} function, which allows it to accept ETH transfers.
-
However, there is no way to withdraw the ETH from the contract (to either a winner or to the owner)
function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
pendingWinnings[msg.sender] = 0;
emit WinningsWithdrawn(msg.sender, amount);
}
function withdrawPlatformFees() external onlyOwner nonReentrant {
uint256 amount = platformFeesBalance;
require(amount > 0, "Game: No platform fees to withdraw.");
platformFeesBalance = 0;
(bool success, ) = payable(owner()).call{value: amount}("");
require(success, "Game: Failed to withdraw platform fees.");
emit PlatformFeesWithdrawn(owner(), amount);
}
@> receive() external payable {}
Risk
Likelihood: Low
-
This only happens if someone is willingly sending ETH to the contract
-
It might happen because the user thinks it can claim the throne by sending ETH to the contract
-
It might happen because the user things it can fund the pot/game by sending ETH to the contract
Impact: Low
This result in loss of ETH, as the owner cannot claim the ETH, the winner does not receive the ETH, and the contract has no way to use the ETH either
Proof of Concept
function testNoEthIsLost() public {
uint256 initialGameBalance = game.getContractBalance();
vm.prank(player1);
(bool success,) = address(game).call{value: 1 ether}("");
require(success, "Failed to send ETH");
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.prank(player2);
game.claimThrone{value: INITIAL_CLAIM_FEE + INITIAL_CLAIM_FEE * FEE_INCREASE_PERCENTAGE / 100}();
vm.warp(block.timestamp + GRACE_PERIOD + 1);
vm.prank(deployer);
game.declareWinner();
vm.prank(player2);
game.withdrawWinnings();
vm.prank(deployer);
game.withdrawPlatformFees();
uint256 finalGameBalance = game.getContractBalance();
console2.log("finalGameBalance", finalGameBalance);
assertEq(finalGameBalance, 0);
}
logs
[FAIL: assertion failed: 1000000000000000000 != 0] testNoEthIsLost() (gas: 367241)
Logs:
finalGameBalance 1000000000000000000
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.54ms (266.46µs CPU time)
Ran 1 test suite in 8.67ms (1.54ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/Game.t.sol:GameTest
[FAIL: assertion failed: 1000000000000000000 != 0] testNoEthIsLost() (gas: 367241)
As can be seen from this test, the 1 ETH is still locked in the contract, without anyone to claim it.
Recommended Mitigation
To prevent this from happening there are 3 options:
Remove the ability to send ETH to the contract (recommended), as this is not part of the functional requirements of the game
- receive() external payable {}
Add the eth amount to the pot
Add a way to withdraw the eth amount by the owner