Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Fees computation in `RockPaperScissors` when a game is tied can leave ETH dust

s3bc40

Summary

Calculation of fees in the RockPaperScissors contract leaves dust when _handleTie() is called. In fact, 10% is taken from the pot for the admin and the rest is split between the players. This is because the division of the pot by 10% for the admin and potentially splitting an odd prize do not take into account the decimal precision of ETH.

// handleTie
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
uint256 refundPerPlayer = (totalPot - fee) / 2;

Vulnerability Details

Impact

This can lead to a situation where the contract has a small amount of ETH left over after the fees are taken. This dust can accumulate over time and lead to a significant amount of ETH being left.

POC

  1. PlayerA create a game with a bet = 0.043784470443721407 ether and playerB joins the game with the same bet: pot = 0.087568940887442814 ether.

  2. Both players play the game and the game ends with a tie.

  3. The contract takes 10% of the pot for the admin: fees = 0.008756894088744281 ether.

  4. The rest of the pot is split between the players: refundPerPlayer = 0.039406023399349266 ether.

  5. The contract returns 0.039406023399349266 ether to each player, so the total pot minus the fees is 0.078812046798698532 ether.

  6. 0.078812046798698532 ether + 0.008756894088744281 ether = 0.087568940887442813 ether, which is not equal to the original pot of 0.087568940887442814 ether.

The difference is 0.000000000000000001 ether, which is the dust.

Tools Used

Foundry test:

// Test handling a tie game
function testAuditTieGameFeeCalculation(uint256 betAmount) public {
// Limit bet amount to 0.01 - 0.05 ETH (to comply with realistic bet amounts)
betAmount = bound(betAmount, 0.01 ether, 0.05 ether);
// Change to 1 turn to make a tie easier to create
vm.prank(playerA);
gameId = game.createGameWithEth{value: betAmount}(1, TIMEOUT);
​
vm.prank(playerB);
game.joinGameWithEth{value: betAmount}(gameId);
​
// Both players play Rock (creates a tie)
uint256 playerABalanceBefore = playerA.balance;
uint256 playerBBalanceBefore = playerB.balance;
​
playTurn(
gameId,
RockPaperScissors.Move.Rock,
RockPaperScissors.Move.Rock
);
​
// Verify game state
(
,
,
,
,
,
,
,
,
,
,
,
,
,
uint8 scoreA,
uint8 scoreB,
RockPaperScissors.GameState state
) = game.games(gameId);
​
assertEq(scoreA, 0);
assertEq(scoreB, 0);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));
​
// Verify both players received half of pot minus fees
uint256 totalPot = betAmount * 2;
uint256 fee = (totalPot * 10) / 100;
uint256 refundPerPlayer = (totalPot - fee) / 2;
​
assertEq(playerA.balance - playerABalanceBefore, refundPerPlayer);
assertEq(playerB.balance - playerBBalanceBefore, refundPerPlayer);
assertEq((refundPerPlayer * 2) + fee, totalPot); // Check total pot
}

Recommendations

Take into account the decimal precision of ETH when calculating fees and splitting the pot.

uint256 fee = ((totalPot * PROTOCOL_FEE_PERCENT * 1e18) / 100) / 1e18;

Consider making a common internal function to handle the fees calculation and splitting the pot. This way, you can ensure that the same logic is used in all cases and avoid any discrepancies. Could be useful for _finishGame() and maybe fixing the unit tests would be great:

// uint256 expectedPrize = ((BET_AMOUNT * 2) * 90) / 100; // 10% fee // Wrong way
uint256 totalPot = BET_AMOUNT * 2;
uint256 fees = ((totalPot * 10) / 100); // 10% fee
uint256 expectedPrize = totalPot - fees; // 90% of the pot minus fees
Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Rounding Error

The tie-handling logic loses one wei due to integer division

m3dython Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Rounding Error

The tie-handling logic loses one wei due to integer division

Support

FAQs

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