Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Funds Sent Directly to Contract Are Permanently Locked

emadsahebix

Root + Impact

Root Cause

The contract implements a receive() function with an empty body. While this function correctly accepts incoming Ether, it fails to update any internal accounting variables like pot or platformFeesBalance. This leaves the received funds untracked by the game's logic.


Impact

This leads to a permanent loss of funds for any user who sends ETH directly to the contract. Because the funds are untracked by the internal accounting, no mechanism exists to withdraw them, causing them to be locked in the contract forever.


Description

  • The contract includes a payable receive() function, allowing it to accept direct Ether transfers. However, the function lacks any logic to account for these funds. This causes any received ETH to become untracked and permanently locked within the contract, leading to a direct loss of funds for the sender.

// ... end of the contract
function getContractBalance() public view returns (uint256) {
return address(this).balance;
}
​
@> receive() external payable {}
​
}

Risk

Likelihood:

  • Reason 1: A user mistakenly sends ETH directly to the contract address, bypassing the claimThrone function.

  • Reason 2: An automated script or another smart contract incorrectly interacts with the game by sending it raw Ether.

Impact:

  • Impact 1: The sender's funds are permanently and irreversibly locked within the contract with no mechanism for recovery.

  • Impact 2: This creates a loss-of-funds scenario that can harm users and damage the protocol's reputation for safety.

Proof of Concept

// POC.t.sol
​
function test_POC_FundsAreLockedWhenSentDirectly() public {
// --- 1. Act: playerA sends 1 ETH directly to the contract ---
uint256 amountToSend = 1 ether;
(bool success, ) = payable(address(game)).call{value: amountToSend}("");
require(success, "Sending ETH failed");
​
// --- 2. Assert: Check if the funds are locked ---
// Check that the contract's total balance has increased.
assertEq(address(game).balance, amountToSend);
​
// Check that the internal accounting variables have NOT changed.
assertEq(game.pot(), 0);
assertEq(game.platformFeesBalance(), 0);
}

Expected Output

git:(main*)$ forge test --match-test test_POC_FundsAreLockedWhenSentDirectly -vvvv
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/POC.t.sol:POCTest
[PASS] test_POC_FundsAreLockedWhenSentDirectly() (gas: 18870)
Traces:
[18870] POCTest::test_POC_FundsAreLockedWhenSentDirectly()
├─ [55] Game::receive{value: 1000000000000000000}()
│ └─ ← [Stop]
├─ [2515] Game::pot() [staticcall]
│ └─ ← [Return] 0
├─ [2472] Game::platformFeesBalance() [staticcall]
│ └─ ← [Return] 0
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 22.31ms (792.20µs CPU time)
​
Ran 1 test suite in 42.74ms (22.31ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)


Recommended Mitigation

The recommended mitigation is to completely disallow direct Ether transfers to the contract. This can be achieved by removing the payable receive() function. If a user accidentally tries to send ETH to the contract, the transaction will revert, protecting them from losing their funds.

- receive() external payable {}

By removing this function, the contract will no longer have a mechanism to accept raw Ether, thus eliminating the vulnerability entirely.

Updates

Appeal created

inallhonesty Lead Judge about 1 month 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.