Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

Anyone can modify the game state to cancelled

[L-2] summary

Anyone can modify the game state to cancelled

vulnerability details

Here we are not creating any game and can still able to emit the RockPaperScissors::GameCancelled for any gameId in the RockPaperScissors::games

POC

function testTimeoutJoinScenario() public {
uint256 gameNum = 1;
vm.prank(playerA);
game.timeoutJoin(gameNum);
}

Test output

Ran 1 test for test/RockPaperScissorsTest.t.sol:RockPaperScissorsTest
[PASS] testTimeoutJoinScenario() (gas: 44160)
Traces:
[44160] RockPaperScissorsTest::testTimeoutJoinScenario()
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [32907] RockPaperScissors::timeoutJoin(1)
│ ├─ emit GameCancelled(gameId: 1)
│ └─ ← [Return]
└─ ← [Return]

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.12ms (48.25µs CPU time)

impact - Low

-> As many real world frontend DEFI applications rely on the events emitted by the contract for a quick confirmation to showup in the UI . if some one has created a new game and the next minute an attacker can emit that gameId as cancelled to trick the new users

-> This will modify the game.state value too

-> This may impact all the conditions wherever we are using checks based on game.state

likelyhood - High

Recommendations

  1. For game.state change , we need to follow this one

We need to mitigate this default enum risk here

  1. Only emit the when the game is really cancelled

function _cancelGame(uint256 _gameId) internal {
Game storage game = games[_gameId];
game.state = GameState.Cancelled;
// Refund ETH to players
if (game.bet > 0) {
(bool successA,) = game.playerA.call{value: game.bet}("");
require(successA, "Transfer to player A failed");
+ emit GameCancelled(_gameId);
if (game.playerB != address(0)) {
(bool successB,) = game.playerB.call{value: game.bet}("");
require(successB, "Transfer to player B failed");
}
}
// Return tokens for token games
if (game.bet == 0) {
if (game.playerA != address(0)) {
winningToken.mint(game.playerA, 1);
+ emit GameCancelled(_gameId);
}
if (game.playerB != address(0)) {
winningToken.mint(game.playerB, 1);
}
}
- emit GameCancelled(_gameId);
}
Updates

Appeal created

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

Missing check for game existence in timeoutJoin

timeoutJoin function allows cancellation and emits a GameCancelled event for any game ID

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

Missing check for game existence in timeoutJoin

timeoutJoin function allows cancellation and emits a GameCancelled event for any game ID

Support

FAQs

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