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

Fees not collected for games that are cancelled

vladich0x

Summary

Fees not collected for games that are cancelled

Vulnerability Details

The internal function RockPaperScissors::_cancelGame() implements core logic behind cancelling games.It starts with transition to final state GameState.Cancelled and further handles the refund of users, sending required ETH amount or minting winner tokens for each participating opponent in the game. But no calculation of ETH fee and further collection is implemented for games with ETH bets.

function _cancelGame(uint256 _gameId) internal {
Game storage game = games[_gameId];
​
game.state = GameState.Cancelled;
​
// Refund ETH to players
if (game.bet > 0) {
// missing ETH fee collection
(bool successA,) = game.playerA.call{value: game.bet}("");
require(successA, "Transfer to player A failed");
​
if (game.playerB != address(0)) {
(bool successB,) = game.playerB.call{value: game.bet}("");
require(successB, "Transfer to player B failed");
}
}
​
// Return tokens for token games
if (game.bet == 0) {
if (game.playerA != address(0)) {
winningToken.mint(game.playerA, 1);
}
if (game.playerB != address(0)) {
winningToken.mint(game.playerB, 1);
}
}
​
emit GameCancelled(_gameId);
}

Impact

Мissing fee collection leads to:

  • protocol requirements are not fulfilled, since the documentation says: 10% protocol fee on all ETH games

  • inconsistent implementation with the other functions RockPaperScissors::_finishGame() and RockPaperScissors::_handleTie(), already implementing calculation and collection of the 10% fee from ETH games

  • lost profits for the game contract and more precise his creator/owner

Tools Used

Manual review
Foundry

PoC

Add the following test to RockPaperScissors.t.sol.

function testNoFeeCollectedOnCancelledGameWithETH() public {
uint256 initBalancePlayerA = playerA.balance;
vm.startPrank(playerA);
vm.expectEmit(true, true, false, true);
emit GameCreated(0, playerA, BET_AMOUNT, TOTAL_TURNS);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
game.cancelGame(gameId);
vm.stopPrank();
​
uint256 finalBalancePlayerA = playerA.balance;
​
assertEq(initBalancePlayerA, finalBalancePlayerA);
}

Recommendations

Implement missing protocol fee collection for cancelled games with ETH bets in function RockPaperScissors::_cancelGame() in order to fulfill the protocol requirements.

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Orphaned ETH due to Unrestricted receive() or Canceled Game

ETH sent directly to the contract via the receive function or after a canceled game becomes permanently locked

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.