Rock Paper Scissors

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

[M-2] Grieving attack: `RockPaperScissors::cancelGame` vulnerable to inflating `WinningToken`.

Description: The _cancelGame() function mints WinningTokens back to playerA and playerB in a token-based game. However, these tokens are not transferred back from contract custody — they are freshly minted, regardless of whether players had already deposited 1 token each when entering the game.

function _cancelGame(uint256 _gameId) internal {
Game storage game = games[_gameId];
game.state = GameState.Cancelled;
// Refund ETH to players
// @audit inspect these external calls more closely
if (game.bet > 0) {
(bool successA,) = game.playerA.call{value: game.bet}("");
require(successA, "Transfer to player A failed");
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
// @audit-medium: potential mint abuse, transfer tokens instead
if (game.bet == 0) {
if (game.playerA != address(0)) {
@> winningToken.mint(game.playerA, 1);
}
if (game.playerB != address(0)) {
@> winningToken.mint(game.playerB, 1);
}
}
emit GameCancelled(_gameId);
}

Impact: A malicious user can grief the protocol by using two wallets to repeatedly create and cancel token-based games, triggering the minting of two new WinningTokens each time. Since deposited tokens aren't returned but freshly minted, this leads to unbounded inflation, undermining WinningToken’s value as a proof of victory.

Proof of Concept: Run the following test in RockPaperScissorsTest.t.sol...

Test
function testInflateWinningTokenOnCancel() public {
// Record initial balances and total supply
uint256 initialSupply = token.totalSupply();
// Create and join a token-based game using the helper
uint256 tokenGameId = createAndJoinTokenGame();
// Cancel the game before any commitments
vm.prank(playerA);
game.cancelGame(tokenGameId);
// Total supply should reflect the 2 newly minted tokens
assertEq(token.totalSupply(), initialSupply + 2, "Total supply should increase by 2 after cancel");
}

Recommended Mitigation:

  1. Instead of minting new tokens on game cancellation, the contract should return the originally deposited tokens using transfer() from its own balance. This ensures no inflation occurs and preserves the integrity of WinningToken as a reward.

Updates

Appeal created

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

Minting Instead of Transferring Staked Tokens

Mints new tokens upon game completion or cancellation for token-based games

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

Minting Instead of Transferring Staked Tokens

Mints new tokens upon game completion or cancellation for token-based games

Support

FAQs

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