Last Man Standing

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

Unguarded `receive()` Function Allows Direct ETH Deposits

Unguarded receive() Function Allows Direct ETH Deposits That Become Permanently Locked in the Contract

Description

  • Normal Behavior: A smart contract should only be able to receive Ether through functions that properly account for the incoming funds, for example by adding them to a prize pot, crediting a user's balance, or assigning them as fees. If a contract is not designed to handle arbitrary ETH transfers, it should reject them.

  • The Issue: The Game contract includes an empty payable receive() function (receive() external payable {}). This makes the contract address a valid destination for direct ETH transfers from any user's wallet. However, the ETH received through this function is not recorded in any of the contract's state variables (pot, platformFeesBalance, or pendingWinnings). Since all withdrawal functions (withdrawWinnings, withdrawPlatformFees) exclusively rely on these state variables to determine payout amounts, any Ether sent directly to the contract bypasses all internal accounting and becomes trapped, with no function available for anyone—including the sender or the contract owner—to withdraw it.

The last code function
// @> receive() external payable {}

Risk

Likelihood: MEDIUM

  • While most users would interact through a designated front-end, it is common for users to find a contract address on an explorer like Etherscan and attempt to interact with it directly. A user could mistakenly send ETH to the contract, believing they are adding to the pot or funding their own play, leading to an accidental but irreversible transaction.

Impact: HIGH

  • The impact of this flaw is the permanent and irreversible loss of user funds. There is no recovery mechanism. This type of vulnerability severely damages user trust and can lead to significant financial loss, making it one of the most critical issues a contract can have.

Proof of Concept

Deploy the Game contract. Note its initial balance is 0 ETH.

  • Note the balance of an external account, User_A.

  • From User_A's wallet, execute a simple transfer of 1 ETH directly to the Game contract address.

  • Observe: The transaction succeeds. The Game contract's balance, as seen on an explorer or by calling address(this).balance, is now 1 ETH. The balance of User_A has decreased by 1 ETH (+ gas).

  • Verify State: Check the contract's state variables: pot, platformFeesBalance, and all entries in pendingWinnings remain unchanged (i.e., they are all 0).

  • Attempt Withdrawal:

    • The contract owner calls withdrawPlatformFees(). The call reverts because platformFeesBalance is 0.

    • User_A calls withdrawWinnings(). The call reverts because pendingWinnings[User_A] is 0.

  • Conclusion: The 1 ETH sent by User_A is permanently locked within the contract, inaccessible by any function or user.


Recommended Mitigation

There are two paths to resolving this, depending on the desired contract behavior. Option A is the most secure and recommended approach.

Option A (Recommended): Disallow Direct ETH Transfers

If the contract should only receive ETH via the claimThrone() function, then all other paths for receiving ETH should be closed. The simplest and safest solution is to remove the receive() function entirely.

Remove the following code:

- receive() external payable {}

Without a receive() or fallback() payable function, any direct ETH transfer sent to the contract will automatically be rejected (the transaction will revert), protecting users from accidentally locking their funds.

Option B (Alternative): Account for Direct Transfers

If the ability to donate or add funds directly to the prize pot is a desired feature, the receive() function must be implemented to handle the funds correctly.

Modify the function to add received funds to the pot:

/**
* @dev Allows users to donate directly to the prize pot.
*/
receive() external payable {
+ pot = pot + msg.value;
// Optionally, emit an event to log the donation
+ // emit DonationReceived(msg.sender, msg.value);
}

This ensures that directly transferred ETH is accounted for and becomes part of the prize that can be won, preventing the funds from being locked. However, this could have unintended consequences on game theory, so Option A is recommended unless this is a specifically designed feature.

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!