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

ETH Sent To Receive Function Can Lock Funds in The Contract And Cause `getContractBalance` To Return An Unintended Value

sneekystryder

ETH Sent To Receive Function Can Lock Funds in The Contract And Cause getContractBalance To Return An Unintended Value

Description

  • The getContractBalance function is intended to return the value of the pot plus platformFees unless payouts are pending. In its logic it returns the balance of the game contract.

  • The receive function accepts ETH without any restrictions or logic to allocate it to the pot or platformFeesBalance, meaning funds sent accidentally or maliciously are not accounted for in the game mechanics and could become locked in the contract, as there is no mechanism to refund or redirect these funds. The getContractBalance function would also return a value not representative of the sum of the pot and platformFeesBalance.

@> receive() external payable {}

Risk

Likelihood:

  • Users or external contracts send ETH directly to the contract's address without invoking the claimThrone function, such as through a direct transfer or a contract's fallback mechanism.

  • Malicious actors intentionally send ETH to the contract to exploit the lack of handling, aiming to lock funds or disrupt game accounting.

Impact:

  • Funds sent to the contract outside of the claimThrone function become unallocated and inaccessible, as they are not added to the pot or platformFeesBalance, potentially locking them in the contract indefinitely.

  • Players or the owner lose trust in the contract's integrity due to untracked funds, leading to reduced participation or disputes over missing ETH.

Proof of Concept

Add this test to Game.t.sol and run with forge test --mt testUnprotectedReceiveLocksFunds

function testUnprotectedReceiveLocksFunds() public {
// Check initial state
uint256 initialBalance = game.getContractBalance();
uint256 initialPot = game.getContractBalance();
uint256 initialPlatformFees = game.platformFeesBalance();
assertEq(initialBalance, 0, "Initial contract balance should be 0");
assertEq(initialPot, 0, "Initial pot should be 0");
assertEq(initialPlatformFees, 0, "Initial platform fees should be 0");
​
// Malicious actor sends 1 ETH directly to the Game contract
vm.prank(maliciousActor);
address(game).call{value: 1 ether}("");
​
// Check state after sending ETH
uint256 finalBalance = game.getContractBalance();
uint256 finalPot = game.pot();
uint256 finalPlatformFees = game.platformFeesBalance();
​
// Asserts that the value returned by getContractBalance() does not equal to the pot and platformFees
assertEq(finalBalance, 1 ether, "Contract balance should increase by 1 ETH");
assert(finalBalance != finalPot + finalPlatformFees);
}

Recommended Mitigation

Have the receive function revert when anyone attempts to send ETH directly to the contract

- receive() external payable {}
+ receive() external payable {
+ revert("Game: Direct ETH transfers not allowed. Use claimThrone.");
+ }
Updates

Appeal created

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