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

Winning Tokens Locked in `RockPaperScissors.sol` after games end in either tie, win or cancelled state

blackgrease

Summary

The RockPaperScissors.sol triggers minting of the Winning Token on token games that are cancelled, won or end in a tie instead of transferring tokens out of the contract resulting in an accumulation of winning tokens within RockPaperScissors that are locked.

Description

The RockPaperScissors.sol allows users to play the game using either Ether or the Winning Token. The Winning Token is only obtainable after winning a game.

Note: token games = games created with createGameWithToken

The Code Issue

When Winning Tokens are used to play, each player must have 1 token, which is transferred into the game. If a game ends by resolution of:

  • cancelled (joining period expired)

  • finished (a winner is found)

  • draw (no winner)

The RockPaperScissors.sol mints players Winning tokens instead of transferring the existing tokens out of the contract. The number of Winning Token minted is dependent on the game resolution.

Tokens Minted according to Game resolution

  1. For token games that are cancelled by creator due to no player joining within the alloted join time, then 1 token is minted in RockPaperScissors::_cancelGame#564 to the creator (PlayerA)...and the Winning Token transferred is left in the contract.

​
if (game.bet == 0) {
if (game.playerA != address(0)) {
winningToken.mint(game.playerA, 1); //@audit: this should be a transfer out
}...

  1. For token games that end in a winner, the Winner is minted 2 tokens. The code area is RockPaperScissors::_finishGame#496. The 2 tokens used by PlayerA and PlayerB are left in the contract.

​
// Handle token prizes - winner gets both tokens
if (game.bet == 0) {
winningToken.mint(_winner, 2);
} ...

  1. For token games that end in a tie, the players are minted 1 token each. The tokens used to join, remain in the contract. The affected area is RockPaperScissors::_handleTie#534

​
// Return tokens for token games
if (game.bet == 0) {
winningToken.mint(game.playerA, 1);
winningToken.mint(game.playerB, 1);
}...

Comparison: Correct Logic

For games that are created with Eth, the expected and correct logic applied, is to transfer assets/funds out of the contract to the respective recipient.

  1. Ether Game that ends in cancelled (RockPaperScissors::_cancelGame#553) game creator is refunded

  2. Ether Game that ends in tie (RockPaperScissors::_handleTie#517) both players are refunded minus the fee.

  3. Ether Games that end with a winner (RockPaperScissors::_finishGame#480) the winner gets both bets minus the fee.

Impact

The impact of this bug is

  1. Locked Winning tokens: As more people participate in the game, there will be an accumulation of locked Winning Token in RockPaperScissors.sol which no one can use. This may affect the value of the Winning Token overtime.

  2. Dilute Value: As more tokens are minted this may dilute the value of the Winning Token as there is a high total supply.

  3. Reputation damage: User trust in the protocol is lowered due to the view in failed development as the contract logic that is accidentally locking tokens.

  4. Unpredictable total supply: As the tokens locked in the contract cannot be circulated, they cannot effectively be included in the total supply therefore making total supply calculations difficult.

This bug can be classed as Medium severity due to no funds being directly lost but as the vulnerability will be triggered on every game created using the Winning Token, it has a high chance of happening. This issue is logical and econmic and cannot be directly exploited.

Tools Used

  • Manual Review

  • Foundry

PoC

The below Proof-of-concept contains 3 test functions

Description

  1. There are 3 players - Player1, Player2, Player3 - that play different games inorder to obtain the Winning Token.

  2. Player2 and Player3 obtain the Winning token after winning games against Player1

  3. Player2 and Player3 are pitted together in token games to demonstrate

  • Tokens are locked after winning [Test function: testTokensLockedInGameViaWin]

  • tokens are locked after a tie. [Test function: testTokensLockedInGameViaTie]

  1. In testTokensLockedInGameViaCancelling, player2 creates and cancels games (after the join timeout) 100 times. After 100 games, the RockPaperScissors contract has 100 locked winning tokens

All 3 test functions can be run using: forge test --mt testTokensLockedInGame -vvv

Note:

  • There are additional internal functions to simulate playerMoves and allow code to be reused across test functions.

  • console.log is used to provide information on the results

Code

​
contract TestLockedWinningToken is Test {
RockPaperScissors game;
WinningToken winningToken;
​
address player1 = makeAddr("player1");
address player2 = makeAddr("player2");
address player3 = makeAddr("player3");
​
uint256 startingBalance = 1 ether;
​
uint256 gameOne;
uint256 gameTwo;
uint256 gameThree;
uint256 gameFour;
​
function setUp() public {
game = new RockPaperScissors();
​
vm.deal(player1, startingBalance);
vm.deal(player2, startingBalance);
vm.deal(player3, startingBalance);
}
​
function _player2joinGame(uint256 _bet) internal {
vm.prank(player2);
game.joinGameWithEth{value: _bet}(gameOne);
}
​
function _player3joinGame(uint256 _gameId, uint256 _bet) internal {
if (_gameId == gameTwo) {
game.joinGameWithEth{value: _bet}(gameTwo);
} else if (_gameId == gameThree) {
winningToken.approve(address(game), 1);
game.joinGameWithToken(gameThree);
} else {
winningToken.approve(address(game), 1);
game.joinGameWithToken(gameFour);
}
}
​
function _player1makeMove(uint256 _gameId) internal {
// Player1 commits
​
vm.prank(player1);
bytes32 saltA = keccak256(abi.encodePacked("salt for player"));
bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Scissors), saltA));
if (_gameId == gameOne) {
game.commitMove(gameOne, commitA);
} else {
game.commitMove(gameTwo, commitA);
}
}
​
function _player2makeMove(uint256 _gameId) internal {
// Player2 commits
vm.prank(player2);
bytes32 saltB = keccak256(abi.encodePacked("salt for player"));
​
if (_gameId == gameOne) {
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltB));
game.commitMove(gameOne, commitB);
} else if (_gameId == gameThree) {
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltB));
game.commitMove(gameThree, commitB);
} else {
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltB));
game.commitMove(gameFour, commitB);
}
}
​
function _player3makeMove(uint256 _gameId) internal {
// Player3 commits
vm.prank(player3);
bytes32 saltB = keccak256(abi.encodePacked("salt for player"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltB));
if (_gameId == gameTwo) {
game.commitMove(gameTwo, commitB);
} else if (_gameId == gameThree) {
game.commitMove(gameThree, commitB);
} else {
game.commitMove(gameFour, commitB);
}
}
​
function _player1Reveal(uint256 _gameId) internal {
vm.prank(player1);
bytes32 saltA = keccak256(abi.encodePacked("salt for player"));
if (_gameId == gameOne) {
game.revealMove(gameOne, 3, saltA);
} else {
game.revealMove(gameTwo, 3, saltA);
}
}
​
function _player2Reveal(uint256 _gameId) internal {
vm.prank(player2);
bytes32 saltB = keccak256(abi.encodePacked("salt for player"));
if (_gameId == gameOne) {
game.revealMove(gameOne, 1, saltB);
} else if (_gameId == gameThree) {
game.revealMove(gameThree, 2, saltB);
} else {
game.revealMove(gameFour, 1, saltB);
}
}
​
function _player3Reveal(uint256 _gameId) internal {
vm.prank(player3);
bytes32 saltB = keccak256(abi.encodePacked("salt for player"));
if (_gameId == gameTwo) {
game.revealMove(gameTwo, 1, saltB);
} else if (_gameId == gameThree) {
game.revealMove(gameThree, 1, saltB);
} else {
game.revealMove(gameFour, 1, saltB);
}
}
​
function _createGameWithTokenAndCancel() internal {
winningToken.approve(address(game), 1);
uint256 turns = 3;
uint256 timeout = 5 minutes;
​
gameTwo = game.createGameWithToken(turns, timeout);
vm.warp(block.timestamp + 25 hours);
game.timeoutJoin(gameTwo);
}
​
modifier Player2Wins() {
//Game One where player 2 successfully wins and gets minted tokens
vm.startPrank(player1);
uint256 minBet = 0.5 ether;
uint256 turns = 1;
uint256 timeout = 6 minutes;
​
gameOne = game.createGameWithEth{value: minBet}(turns, timeout);
vm.stopPrank();
​
_player2joinGame(minBet);
​
_player2makeMove(gameOne);
_player1makeMove(gameOne);
​
_player2Reveal(gameOne);
_player1Reveal(gameOne);
​
uint256 expectedWinningBalance = 1.4 ether;
assertEq(address(player2).balance, expectedWinningBalance); //confirming Player 2 successfully recieved prize money
​
winningToken = game.winningToken();
uint256 expectedplayer2TokenBalance = winningToken.balanceOf(player2);
assertEq(expectedplayer2TokenBalance, 1);
_;
}
​
modifier Player3Wins() {
//Game One where player 2 successfully wins and gets minted tokens
vm.startPrank(player1);
uint256 minBet = 0.5 ether;
uint256 turns = 1;
uint256 timeout = 6 minutes;
​
gameTwo = game.createGameWithEth{value: minBet}(turns, timeout);
vm.stopPrank();
​
vm.startPrank(player3);
_player3joinGame(gameTwo, minBet);
vm.stopPrank();
​
_player3makeMove(gameTwo);
_player1makeMove(gameTwo);
​
_player3Reveal(gameTwo);
_player1Reveal(gameTwo);
​
uint256 expectedWinningBalance = 1.4 ether;
assertEq(address(player3).balance, expectedWinningBalance); //confirming Player 2 successfully recieved prize money
​
winningToken = game.winningToken();
uint256 expectedplayer2TokenBalance = winningToken.balanceOf(player3);
assertEq(expectedplayer2TokenBalance, 1);
_;
}
​
function testTokensLockedInGameViaWin() public Player2Wins Player3Wins {
uint256 gameStartingTokens = winningToken.balanceOf(address(game));
uint256 player2startingTokens = winningToken.balanceOf(player2);
uint256 player3startingTokens = winningToken.balanceOf(player3);
​
//Player 2 and Player 3 both have 1 wining token each aand can now play against each other. Player 2 Wins
vm.startPrank(player2);
uint256 minBet = 0.5 ether;
uint256 turns = 1;
uint256 timeout = 6 minutes;
winningToken.approve(address(game), 1);
gameThree = game.createGameWithToken(turns, timeout);
vm.stopPrank();
​
vm.startPrank(player3);
_player3joinGame(gameThree, minBet);
vm.stopPrank();
​
_player3makeMove(gameThree);
​
_player2makeMove(gameThree);
​
_player3Reveal(gameThree);
_player2Reveal(gameThree);
​
//Balances
​
uint256 gameEndingTokens = winningToken.balanceOf(address(game));
assert(gameEndingTokens > gameStartingTokens);
​
uint256 player2endingTokens = winningToken.balanceOf(player2);
uint256 player3endingTokens = winningToken.balanceOf(player3);
​
//Logging
​
console.log("Game starts with this many tokens: ", gameStartingTokens);
console.log("Tokens Player 2 starts with: ", player2startingTokens);
console.log("Tokens Player 3 starts with: ", player3startingTokens);
console.log("---PLAYER2 WINS---");
console.log("Tokens Player 2 ends with: ", player2endingTokens);
console.log("Tokens Player 3 ends with: ", player3endingTokens);
console.log("Tokens locked in contract: ", gameEndingTokens);
}
​
function testTokensLockedInGameViaTie() public Player2Wins Player3Wins {
uint256 gameStartingTokens = winningToken.balanceOf(address(game));
uint256 player2startingTokens = winningToken.balanceOf(player2);
uint256 player3startingTokens = winningToken.balanceOf(player3);
​
//Player 2 and Player 3 both have 1 wining token each aand can now play against each other. Ends in Tie
​
vm.startPrank(player2);
uint256 minBet = 0.5 ether;
uint256 turns = 1;
uint256 timeout = 6 minutes;
winningToken.approve(address(game), 1);
gameFour = game.createGameWithToken(turns, timeout);
vm.stopPrank();
​
vm.startPrank(player3);
_player3joinGame(gameFour, minBet);
vm.stopPrank();
​
_player3makeMove(gameFour);
​
_player2makeMove(gameFour);
​
_player3Reveal(gameFour);
_player2Reveal(gameFour);
​
//Balances
​
uint256 gameEndingTokens = winningToken.balanceOf(address(game));
assert(gameEndingTokens > gameStartingTokens);
​
uint256 player2endingTokens = winningToken.balanceOf(player2);
uint256 player3endingTokens = winningToken.balanceOf(player3);
​
//Logging
​
console.log("Game starts with this many tokens: ", gameStartingTokens);
console.log("Tokens Player 2 starts with: ", player2startingTokens);
console.log("Tokens Player 3 starts with: ", player3startingTokens);
console.log("---PLAYER TIE---");
console.log("Tokens Player 2 ends with: ", player2endingTokens);
console.log("Tokens Player 3 ends with: ", player3endingTokens);
console.log("Tokens locked in contract: ", gameEndingTokens);
}
​
function testTokensLockedInGameViaCancelling() public Player2Wins {
//Repeat the act of cancelling 100 times to demonstrate locked tokens
uint256 numberOfTokens = 100;
for (uint256 x = 0; x < numberOfTokens; x++) {
vm.startPrank(player2);
_createGameWithTokenAndCancel();
vm.stopPrank();
}
​
uint256 newTokenBalance = winningToken.balanceOf(player2);
uint256 gameTokenBalance = winningToken.balanceOf(address(game));
​
//Logging
console.log("Token Balance of Player2 after 100 cancelled Games: ", newTokenBalance);
console.log("Token Balance of RockPaperScissors after 100 cancelled Games: ", gameTokenBalance);
}
}
​

Mitigation

To mitigate this vulnerability, the contract should

  1. use safeTransferFrom to move the tokens out of the RockPaperScissors contract instead of using mint().

  2. Alternatively, have a system to burn tokens in the contract, either using burn or burnFrom .

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.