Submission Details
Severity:
Precision Loss in Tie Scenario Causes Permanent ETH Lock in Contract
Summary
The RockPaperScissors contract suffers from precision loss when calculating refunds during a tie scenario. The issue occurs when the total pot is not evenly divisible by 2 after the protocol fee is taken, causing small amounts of ETH to remain permanently locked in the contract. While each instance results in minimal ETH loss (typically 1 wei), the cumulative impact could be significant over many games, especially with higher bet amounts that result in odd-number calculations
Vulnerability Details
The vulnerability occurs in the _handleTie() function, specifically in the following calculation:
// Calculate protocol fee (10% of total pot)
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
uint256 refundPerPlayer = (totalPot - fee) / 2;
When the value (totalPot - fee) is odd, integer division by 2 results in a truncated value. The lost wei (from truncation) remains in the contract without any mechanism to recover it.
Proof of Concept
function testPrecisionLossInTieScenario() public {
// Use a bet amount that would create an odd value after fee
// Example: 10000000000000005 wei (slightly above 0.01 ETH)
uint256 betAmount = 0.01 ether + 5 wei;
vm.prank(playerA);
uint256 id = game.createGameWithEth{value: betAmount}(
TOTAL_TURNS,
TIMEOUT
);
vm.prank(playerB);
game.joinGameWithEth{value: betAmount}(id);
// Track balances before
uint256 contractBalanceBefore = address(game).balance;
uint256 playerABalanceBefore = playerA.balance;
uint256 playerBBalanceBefore = playerB.balance;
uint256 feesBefore = game.accumulatedFees();
// Play to a tie (rock vs rock for all turns)
playTurn(id, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Rock);
playTurn(id, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Rock);
playTurn(id, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Rock);
// Calculate expected values
uint256 totalPot = betAmount * 2;
uint256 expectedFee = (totalPot * 10) / 100;
uint256 expectedRefundPerPlayer = (totalPot - expectedFee) / 2;
// Check actual values
uint256 playerARefund = playerA.balance - playerABalanceBefore;
uint256 playerBRefund = playerB.balance - playerBBalanceBefore;
uint256 feeCollected = game.accumulatedFees() - feesBefore;
uint256 totalRefunded = playerARefund + playerBRefund + feeCollected;
// Check for precision loss
uint256 contractBalanceAfter = address(game).balance;
uint256 remainingDust = contractBalanceAfter -
(contractBalanceBefore - totalRefunded);
console.log("--- Testing tie scenario with bet amount:", betAmount, "wei ---");
console.log("Total pot:", totalPot);
console.log("Expected fee:", expectedFee);
console.log("Expected refund per player:", expectedRefundPerPlayer);
console.log("Actual player A refund:", playerARefund);
console.log("Actual player B refund:", playerBRefund);
console.log("Actual fee collected:", feeCollected);
console.log("Total refunded:", totalRefunded);
if (totalRefunded != totalPot) {
console.log("PRECISION LOSS DETECTED!");
console.log("Missing wei:", totalPot - totalRefunded);
console.log("Dust remaining in contract:", remainingDust);
}
}
Result
--- Testing tie scenario with bet amount: 10000000000000005 wei ---
Total pot: 20000000000000010
Expected fee: 2000000000000001
Expected refund per player: 9000000000000004
Actual player A refund: 9000000000000004
Actual player B refund: 9000000000000004
Actual fee collected: 2000000000000001
Total refunded: 20000000000000009
PRECISION LOSS DETECTED!
Missing wei: 1
Dust remaining in contract: 2000000000000001
Traces:
[530033] RockPaperScissorsTest::testPrecisionLossInTieScenario()
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [184325] RockPaperScissors::createGameWithEth{value: 10000000000000005}(3, 600)
│ ├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 10000000000000005 [1e16], totalTurns: 3)
│ └─ ← [Return] 0
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [24919] RockPaperScissors::joinGameWithEth{value: 10000000000000005}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [2455] RockPaperScissors::accumulatedFees() [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [47751] RockPaperScissors::commitMove(0, 0x2ce9e0c6cee52fff8defaf803e3d42570ca0a102e22ce2de7049458b2e57f4bd)
│ ├─ emit MoveCommitted(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [46119] RockPaperScissors::commitMove(0, 0xc9b1d04a9cb0262bf4cc638482b594250b9877d0c0d9aa13fd38efca1df6a449)
│ ├─ emit MoveCommitted(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [4375] RockPaperScissors::revealMove(0, 1, 0xb744b7af2ebddcdb6fdac9e865330c27a0287b13a1c9d7fa8098cde1928d587c)
│ ├─ emit MoveRevealed(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], move: 1, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [7470] RockPaperScissors::revealMove(0, 1, 0x65620014c3c7775136e33ca26bd4fe9e6301884391a12f181a6c7c133b1f8e69)
│ ├─ emit MoveRevealed(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], move: 1, currentTurn: 1)
│ ├─ emit TurnCompleted(gameId: 0, winner: 0x0000000000000000000000000000000000000000, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [23566] RockPaperScissors::commitMove(0, 0x2ce9e0c6cee52fff8defaf803e3d42570ca0a102e22ce2de7049458b2e57f4bd)
│ ├─ emit MoveCommitted(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], currentTurn: 2)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [23993] RockPaperScissors::commitMove(0, 0xc9b1d04a9cb0262bf4cc638482b594250b9877d0c0d9aa13fd38efca1df6a449)
│ ├─ emit MoveCommitted(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], currentTurn: 2)
│ └─ ← [Return]
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [4375] RockPaperScissors::revealMove(0, 1, 0xb744b7af2ebddcdb6fdac9e865330c27a0287b13a1c9d7fa8098cde1928d587c)
│ ├─ emit MoveRevealed(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], move: 1, currentTurn: 2)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [7470] RockPaperScissors::revealMove(0, 1, 0x65620014c3c7775136e33ca26bd4fe9e6301884391a12f181a6c7c133b1f8e69)
│ ├─ emit MoveRevealed(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], move: 1, currentTurn: 2)
│ ├─ emit TurnCompleted(gameId: 0, winner: 0x0000000000000000000000000000000000000000, currentTurn: 2)
│ └─ ← [Return]
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [23566] RockPaperScissors::commitMove(0, 0x2ce9e0c6cee52fff8defaf803e3d42570ca0a102e22ce2de7049458b2e57f4bd)
│ ├─ emit MoveCommitted(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], currentTurn: 3)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [23993] RockPaperScissors::commitMove(0, 0xc9b1d04a9cb0262bf4cc638482b594250b9877d0c0d9aa13fd38efca1df6a449)
│ ├─ emit MoveCommitted(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], currentTurn: 3)
│ └─ ← [Return]
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [4375] RockPaperScissors::revealMove(0, 1, 0xb744b7af2ebddcdb6fdac9e865330c27a0287b13a1c9d7fa8098cde1928d587c)
│ ├─ emit MoveRevealed(gameId: 0, player: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], move: 1, currentTurn: 3)
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [44933] RockPaperScissors::revealMove(0, 1, 0x65620014c3c7775136e33ca26bd4fe9e6301884391a12f181a6c7c133b1f8e69)
│ ├─ emit MoveRevealed(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], move: 1, currentTurn: 3)
│ ├─ emit TurnCompleted(gameId: 0, winner: 0x0000000000000000000000000000000000000000, currentTurn: 3)
│ ├─ emit FeeCollected(gameId: 0, feeAmount: 2000000000000001 [2e15])
│ ├─ [0] playerA::fallback{value: 9000000000000004}()
│ │ └─ ← [Stop]
│ ├─ [0] playerB::fallback{value: 9000000000000004}()
│ │ └─ ← [Stop]
│ ├─ emit GameFinished(gameId: 0, winner: 0x0000000000000000000000000000000000000000, prize: 0)
│ └─ ← [Return]
├─ [455] RockPaperScissors::accumulatedFees() [staticcall]
│ └─ ← [Return] 2000000000000001 [2e15]
├─ [0] console::log("--- Testing tie scenario with bet amount:", 10000000000000005 [1e16], "wei ---") [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Total pot:", 20000000000000010 [2e16]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Expected fee:", 2000000000000001 [2e15]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Expected refund per player:", 9000000000000004 [9e15]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Actual player A refund:", 9000000000000004 [9e15]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Actual player B refund:", 9000000000000004 [9e15]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Actual fee collected:", 2000000000000001 [2e15]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Total refunded:", 20000000000000009 [2e16]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("PRECISION LOSS DETECTED!") [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Missing wei:", 1) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Dust remaining in contract:", 2000000000000001 [2e15]) [staticcall]
│ └─ ← [Stop]
└─ ← [Return]
The test case demonstrates this issue by creating a game with a bet of 0.01 ether + 5 wei, resulting in a total pot of 20000000000000010 wei. After the 10% fee of 2000000000000001 wei is deducted, the remaining amount 18000000000000009 wei is divided by 2, leaving 9000000000000004 wei for each player. This calculation loses 1 wei to truncation, which remains trapped in the contract with no mechanism to extract it.
Impact
Financial: Small amounts of ETH become permanently locked in the contract. While each instance may only lose 1 wei, the cumulative effect across many games and potentially larger bet amounts could be significant.
Accounting Discrepancy: The contract's ETH balance will grow over time due to accumulated dust, which could lead to accounting inconsistencies between the contract's actual balance and what is recorded in its state variables.
Trust: Users may lose trust in the system when they notice that the total amount distributed back to players is less than what was contributed.
Tools Used
Manual code review
Forge/Foundry for test simulation
Recommendations
use math library to handle mathematical operation
Add the Remainder to Protocol Fee:
Updates
Appeal created
m3dython 5 months ago
Submission Judgement Published Assigned finding tags:
Rounding Error
The tie-handling logic loses one wei due to integer division
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.