Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

Rounding Error in ETH Tie Games Leads to Stuck Funds

Summary

A rounding error in the ETH tie game resolution logic causes small amounts of ETH (up to 1 wei per tied game) to become permanently stuck in the contract. This occurs due to integer division truncation when splitting refunds between players.

Vulnerability Details

Root Cause:
In RockPaperScissors::_handleTie, the contract calculates player refunds as:

https://github.com/CodeHawks-Contests/2025-04-rock-paper-scissors/blob/25cf9f29c3accd96a532e416eee6198808ba5271/src/RockPaperScissors.sol#L521C13-L521C60

uint256 refundPerPlayer = (totalPot - fee) / 2;

When (totalPot - fee) is an odd number, 1 wei remains unallocated and stays locked in the contract.

Proof of Concept:

Add this function test:

// Test that no ETH gets stuck in contract during tie games
function test_TieGameNoStuckEth() public {
// Setup game with odd bet amount to force rounding
uint256 oddBetAmount = 0.101 ether; // 101000000000000000 wei
vm.prank(playerA);
gameId = game.createGameWithEth{value: oddBetAmount}(1, TIMEOUT); // 1-turn game
vm.prank(playerB);
game.joinGameWithEth{value: oddBetAmount}(gameId);
// Record contract balance before the tie
uint256 contractBalanceBefore = address(game).balance;
// Force a tie (both players play same move)
playTurn(gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Rock);
// Calculate expected values
uint256 totalPot = oddBetAmount * 2;
uint256 fee = (totalPot * 10) / 100; // 10% fee
uint256 refundPerPlayer = (totalPot - fee) / 2;
uint256 remainder = (totalPot - fee) % 2; // Should be 1 wei
// Verify players got correct refunds
assertEq(playerA.balance, 10 ether - oddBetAmount + refundPerPlayer);
assertEq(playerB.balance, 10 ether - oddBetAmount + refundPerPlayer);
// Verify contract balance changed correctly
// Initial balance: 2 * oddBetAmount (from create+join)
// After refunds: should only keep fee + remainder
uint256 expectedContractBalance = fee + remainder;
console.log("Expected contract balance: ", expectedContractBalance);
console.log("address(game).balance: ", address(game).balance);
assertEq(address(game).balance, contractBalanceBefore - (refundPerPlayer * 2));
// Verify remainder was added to fees
assertEq(game.accumulatedFees(), fee + remainder);
}

run the test: forge test --match-test test_TieGameNoStuckEth -vvv

Impact

  • Stuck Funds: Repeated tied games cause accumulation of dust ETH (1 wei per occurrence).

  • Manual Recovery Required: Admin must manually recover these funds, adding operational overhead.

  • Protocol Integrity: Unaccounted ETH may lead to accounting discrepancies over time.

Tools Used

Recommendations

Add the leftover wei to the protocol fees to ensure no ETH is stranded:

uint256 refundPerPlayer = (totalPot - fee) / 2;
+ accumulatedFees += (totalPot - fee) % 2; // Captures remainder (0 or 1 wei)
  • Ensures 100% of ETH is either refunded or collected as fees.

  • Eliminates need for manual recovery.

  • Maintains precise financial tracking.

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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