Submission Details
Severity:
[M-1] Using `mint` instead of `transferFrom` in `RockPaperScissors::_finishGame` function inflates total supply of the winning token over time
**Description:** Whenever a token game is finished and the winner is decided, the protocol mints the winner new tokens instead of transfering the tokens bet from both players on the game. This happens in `RockPaperScissors::_finishGame`, `RockPaperScissors::_handleTie` and `RockPaperScissors::_cancelGame`. In the `_handleTie` and `_cancelGame` functions instead of just returning the tokens to the players, they are minted new ones.
**Impact:** Total supply inflation of the winning token of the protocol has a very high likelihood of happening since any time a token game is finished protocol mints instead of transfering.
**Proof of Concept:** After the game ends we can see that the total supply after the token game is higher than the one before the game which inflates protocol's total supply.
<details>
<summary>PoC</summary>
Put this in the `RockPaperScissorsTest.t.sol`:
```javascript
function testMintingInsteadOfTransferingInflatesTotalSupply() public {
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);
uint256 totalSupplyOfProtocolBefore = token.totalSupply();
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));
uint256 totalSupplyOfProtocolAfter = token.totalSupply();
// Verify winner received 2 tokens (both players' stakes)
assertEq(token.balanceOf(playerA) - tokenBalanceABefore, 2);
assertGt(totalSupplyOfProtocolAfter, totalSupplyOfProtocolBefore);
}
```
</details>
**Recommended Mitigation:** Instead of using `mint` in the functions enumerated above, `transferFrom` should instead be used.
Example:
```diff
function _finishGame(uint256 _gameId, address _winner) internal {
.
.
.
if (game.bet == 0) {
// Mint a winning token
- winningToken.mint(_winner, 2);
+ winningToken.transferFrom(address(this), _winner, 2);
.
.
.
```
Updates
Appeal created
m3dython about 2 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 about 2 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.