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

Two winning tokens are locked in the game smart contract for each token game

accessdenied

Summary

For each token game, there are two winnings/bet tokens locked inside the game smart contract.

Vulnerability Details

When a game token is created with the function createGameWithToken, the winning token is transfered to the game smart contract.

// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);

This is also the case when a player B wants to join the game by calling the function joinGameWithToken

// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);

But when the winner is chosen, instead of transferred the tokens inside the contract to the winner, the game mints new tokens in the function _finishTokens.

// Handle token prizes - winner gets both tokens
if (game.bet == 0) {
// Mint a winning token
winningToken.mint(_winner, 2);
} else {
// Mint a winning token for ETH games too
winningToken.mint(_winner, 1);
}

There is the same problem in the function _cancelGame

if (game.bet == 0) {
if (game.playerA != address(0)) {
winningToken.mint(game.playerA, 1);
}
if (game.playerB != address(0)) {
winningToken.mint(game.playerB, 1);
}
}

Impact

This design generates several problems:

  • The two Winning tokens transferred are locked inside the game contract

  • The total supply of the token is "artificially" increased

  • The game smart contract must have the minter role on the token, which is not necessary for this use case. It reduces also the possibility in the futur to use another ERC-20 token (e.g USDC) as a winning token for a token game.

function testCompleteTokenGame() public {
assertEq(token.balanceOf(address(game)), 0);
gameId = createAndJoinTokenGame();
​
// First turn: A=Paper, B=Rock (A wins)
playTurn(gameId, RockPaperScissors.Move.Paper, RockPaperScissors.Move.Rock);
​
// Second turn: A=Rock, B=Scissors (A wins)
playTurn(gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Scissors);
​
// Third turn: A=Paper, B=Scissors (B wins, but A still has more points)
uint256 tokenBalanceABefore = token.balanceOf(playerA);
​
playTurn(gameId, RockPaperScissors.Move.Paper, RockPaperScissors.Move.Scissors);
​
// Verify game state
(,,,,,,,,,,,,, uint8 scoreA, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);
​
assertEq(scoreA, 2);
assertEq(scoreB, 1);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));
​
// Verify winner received 2 tokens (both players' stakes)
assertEq(token.balanceOf(playerA) - tokenBalanceABefore, 2);
​
// token are locked in the contract
assertEq(token.balanceOf(address(game)), 2);
}

Tools Used
Manual analysis (My brain) + Foundry

Recommendations

Use safeTransferFrom from OpenZeppelin to transfer the token from the game smart contract to the winner instead of minting new tokens.

Updates

Appeal created

m3dython Lead Judge about 2 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.