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

`WinningToken` Tokens Are Locked in the `RockPaperScissors` Contract Without the Possibility to Transfer or Burn Them.

jfornells

Descrption:

The RockPaperScissors contract allows players to stake WinningToken tokens when joining a game via the joinGameWithToken() function. These tokens are transferred into the contract using transferFrom() and are held by the contract during gameplay.

However, at the conclusion of the game-whether it ends in a win (_finishGame()), draw (_handleTie()), or is cancelled (_cancelGame())-, the originally staked tokens are neither returned to players nor burned. Instead, the contract simply mints new WinningToken tokens to the winner (or to both players in a draw), while the original tokens remain indefinitely locked inside the contract.

Although WinningToken inherits from ERC20Burnable, the burn() functionality is not utilized. Moreover, the contract lacks any admin function that could reclaim or manage the accumulated tokens. Over time, this behavior leads to silent inflation and unnecessary token accumulation, which could impact the token economy and contract sustainability.

Impact

  • Tokens transferred by players are never recovered or destroyed, effectively becoming inaccessible.

  • The system silently inflates the WinningToken supply by minting new tokens for winners, while keeping the original staked tokens in contract storage.

  • This may mislead users or third parties inspecting totalSupply, and could lead to long-term sustainability issues.

  • Accumulation of unused tokens within the contract could require emergency administrative intervention in the future.

Proof Of Concept:

Test for the cancelGame() function

  1. Capture the balances of playerA and the RockPaperScissors contract.

  2. playerA approves the RockPaperScissors contract for 1 token.

  3. playerA creates a new game.

  4. Capture the balances during the game.

  5. playerA cancels the game.

  6. Capture the balances after the game.

function test_blocked_WinningTokens() public {
vm.startPrank(playerA);
​
uint256 playerABalanceBefore = token.balanceOf(playerA);
uint256 gameBalanceBefore = token.balanceOf(address(game));
​
console2.log("PlayerA Balance Before...", playerABalanceBefore);
console2.log("Game Balance Before......", gameBalanceBefore);
console2.log("--------------------------------");
token.approve(address(game), 1);
uint256 gameId = game.createGameWithToken(3, 5 minutes);
​
uint256 playerABalance = token.balanceOf(playerA);
uint256 gameBalance = token.balanceOf(address(game));
​
console2.log("PlayerA Balance During...", playerABalance);
console2.log("Game Balance During......", gameBalance);
console2.log("--------------------------------");
​
game.cancelGame(gameId);
uint256 playerABalanceAfter = token.balanceOf(playerA);
uint256 gameBalanceAfter = token.balanceOf(address(game));
​
console2.log("PlayerA Balance After....", playerABalanceAfter);
console2.log("Game Balance After.......", gameBalanceAfter);
}

This test checks balances before and after creating and canceling a game. When playerA transfers 1 token to the RockPaperScissors contract, it remains locked inside the contract even after the game is canceled.

Although playerA is minted a new token during the cancellation process, the originally transferred token is neither returned nor burned, causing tokens to accumulate in the contract over time. This same behavior is present in tie scenarios via _handleTie() and when the game ends via _finishGame(), where new tokens are minted, but previously transferred tokens are never handled.

This progressive accumulation of locked tokens may lead to sustainability issues and could require future administrative intervention or additional token management logic.

Logs:
PlayerA Balance Before... 10
Game Balance Before...... 100
--------------------------------
PlayerA Balance During... 9
Game Balance During...... 101
--------------------------------
PlayerA Balance After.... 10
Game Balance After....... 101

Tools Used

Manual Review and Foundry

Recommendations

To prevent token accumulation within the contract, we recommend the following:

Burn the originally staked tokens upon game resolution (_cancelGame, _handleTie, _finishGame), instead of leaving them locked.
While the WinningToken contract inherits from ERC20Burnable, it does not expose any burnFrom()-like functionality for external contracts to burn held tokens.
Implement a restricted burn() function callable by the RockPaperScissors contract (or its owner) to cleanly dispose of these locked tokens.

Add a burn function to the WinningToken contract:

contract WinnigToken {
...
+ function burn(address from, uint256 amount) external onlyOwner {
+ require(amount > 0, "Amount must be greater than 0");
+ _burn(from, amount);
+ }
...
}

Update game resolution functions:

function _finishGame(...) internal {
...
// Handle token prizes - winner gets both tokens
if (game.bet == 0) {
// Mint a winning token
+ winningToken.burn(address(this), 2);
winningToken.mint(_winner, 2);
} else {
// Mint a winning token for ETH games too
winningToken.mint(_winner, 1);
}
...
}
function _handleTie(...) internal {
...
// Return tokens for token games
if (game.bet == 0) {
+ winningToken.burn(address(this), 1);
winningToken.mint(game.playerA, 1);
+ winningToken.burn(address(this), 1);
winningToken.mint(game.playerB, 1);
}
...
}
function _cancelGame(uint256 _gameId) internal {
...
// Return tokens for token games
if (game.bet == 0) {
if (game.playerA != address(0)) {
+ winningToken.burn(address(this), 1);
winningToken.mint(game.playerA, 1);
}
if (game.playerB != address(0)) {
+ winningToken.burn(address(this), 1);
winningToken.mint(game.playerB, 1);
}
}
...
}

To prevent WinningToken from being permanently locked due to direct transfers, implement an admin-only recovery function:

+ function rescueStuckTokens(uint256 amount) external onlyOwner {
+ uint256 contractBalance = winningToken.balanceOf(address(this));
+ require(amount <= contractBalance, "Insufficient token balance");
+ winningToken.transfer(adminAddress, amount);
}
Updates

Appeal created

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