Submission Details
Severity:
Unneccesary `WinningToken` minting in `_finishGame` internal function.
Description
After revealing commit moves by both of the player the `_determineWinner` internal function will determine the winner for current turn. But there is another function which will finish game which is `_finishGame`. but in this function the protocol is minting unneccesary winning tokens to the winner instead of transfer token from contract to the winner which initially player has transfered to thegame contract at a time of joining.
```javascript
// Handle token prizes - winner gets both tokens
if (game.bet == 0) {
// Mint a winning token
@> winningToken.mint(_winner, 2);
} else {
```
This minting will results in increasing total supply of the winning token and the actual deposited token will stuck in the game permanently.
This unneccesary minting is also present in `_handleTie`, `_cancelGame` internal function also.
Impact
There are two impacts according to me.
1. The initial transfered token while joining into game will stuck in the game.
2. The total supply of the Winning token will increse unneccesary.
Proof of Concept
Add this function into the test file.
```javascript
function test_tokenWillStuckIncontractDueToUnnecceseryMintingOfWinningToken() public {
// First create a game with token
vm.startPrank(playerA);
token.approve(address(game), 1);
gameId = game.createGameWithToken(1, TIMEOUT);
vm.stopPrank();
// Now join the game with token
vm.startPrank(playerB);
token.approve(address(game), 1);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithToken(gameId);
vm.stopPrank();
// console.log("Total Supply:", token.totalSupply());
// Verify token transfer
assertEq(token.balanceOf(playerA), 9);
assertEq(token.balanceOf(playerB), 9);
assertEq(token.balanceOf(address(game)), 2);
// Player A commits
bytes32 saltA = keccak256(abi.encodePacked("salt for player A"));
bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));
vm.prank(playerA);
vm.expectEmit(true, true, false, true);
emit MoveCommitted(gameId, playerA, 1);
game.commitMove(gameId, commitA);
// Player B commits
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltB));
vm.prank(playerB);
vm.expectEmit(true, true, false, true);
emit MoveCommitted(gameId, playerB, 1);
game.commitMove(gameId, commitB);
// Verify game state
(,,,,,,,,, bytes32 storedCommitA, bytes32 storedCommitB,,,,, RockPaperScissors.GameState state) =
game.games(gameId);
assertEq(storedCommitA, commitA);
assertEq(storedCommitB, commitB);
vm.prank(playerA);
game.revealMove(gameId, 1, saltA);
vm.prank(playerB);
game.revealMove(gameId, 2, saltB);
assertEq(token.balanceOf(playerB), 11);
assertEq(token.balanceOf(address(game)), 2);
}
```
Recommendations
Transfer the token from the game contract to the winner or to the appropriate player instead of mining new tokens to the player.
make this update in `_finishGame` internal function.
```diff
if (game.bet == 0) {
// Mint a winning token
- winningToken.mint(_winner, 2);
+ winningToken.transfer(_winner, 2);
} else {
```
also update the related function as same as `_finishGame` function which mint unneccesary tokens to the player.
Updates
Appeal created
m3dython 4 months ago
Submission Judgement Published Assigned finding tags:
Minting Instead of Transferring Staked Tokens
Mints new tokens upon game completion or cancellation for token-based games
m3dython 4 months ago
Submission Judgement Published Assigned finding tags:
Minting Instead of Transferring Staked Tokens
Mints new tokens upon game completion or cancellation for token-based games
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.