Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Impact: low
Likelihood: low
Invalid

ETH stuck in contract

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)

// Withdraw only pendingWinnings
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);
}
// Withdraw only stored platform fees
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);
}
// Allow to receive ETH from anyone
@> 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();
// Simulate player 1 ending 1 ETH
vm.prank(player1);
(bool success,) = address(game).call{value: 1 ether}("");
require(success, "Failed to send ETH");
// Simulate a game round with normal ETH transfer flows
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();
// Withdraw winnings and fees
vm.prank(player2);
game.withdrawWinnings();
vm.prank(deployer);
game.withdrawPlatformFees();
// Final balance should be 0, as all ETH should be claimed by the owner OR the winner
// However it is stuck in the contract
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:

  1. 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 {}
  1. Add the eth amount to the pot

  2. Add a way to withdraw the eth amount by the owner

Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Direct ETH transfers - User mistake

There is no reason for a user to directly send ETH or anything to this contract. Basic user mistake, info, invalid according to CH Docs.

Support

FAQs

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

Give us feedback!