TwentyOne
First Flight #29
Beginner FriendlyGameFiFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Missing check for sufficient prize pool for player in `TwentyOne::startGame` may cause function `TwentyOne::endGame` to revert and prevent payout to winning player and restart of new game

trashpirate

Summary

Winning players can only be paid out when the contract balance is sufficient to pay out an additional 1 ETH on top of the player's deposit. There should be a check in the function TwentyOne::startGame to ensure sufficient funds are availalbe for payout before starting a new game.

function startGame() public payable returns (uint256) {
address player = msg.sender;
require(msg.value >= 1 ether, "not enough ether sent");
@> // missing check for contract balance
initializeDeck(player);
uint256 card1 = drawCard(player);
uint256 card2 = drawCard(player);
addCardForPlayer(player, card1);
addCardForPlayer(player, card2);
return playersHand(player);
}

Vulnerability Details

The description in the README states that users can wager 1 ETH and that Winning players double their wager, while losing players forfeit their initial bet. This means that winning players should receive 2 ETH when the game ends. Unless a player always looses before a win, the contract needs to be funded with sufficient ETH such that the payout of a win is successful. Thus, if the contract balance is less than 2 ETH when a player wins, the function TwentyOne::endGame will fail and revert, the player won't receive the payout and won't e able to restart a game until the function TwentyOne::endGame is successfully called. A check should be added to the function TwentyOne::startGame to ensure that the contract balance is sufficient to pay out a winning player before starting a new game. Given that multiple players can play the game at the same time, the check should only allow a new game to start if the contract balance is sufficient to pay out all possible winning players.

Proof of Concept

  1. Player starts game with 1 ETH

  2. Player calls TwentyOne::call

  3. Player wins the game

  4. Function TwentyOne::call calls TwentyOne::endGame to pay out the player

  5. Function TwentyOne::endGame fails and reverts because the contract balance is less than 2 ETH ([OutOfFunds])

  6. No payout for winner, new game can't be started until TwentyOne::endGame is successfully called.

Code:

Place following code into TwentyOne.t.sol:

function test_CallReverts_PlayerWinsFirstGame() public {
// start game
vm.prank(player1);
twentyOne.startGame{value: 1 ether}();
​
// playersHand == 14
console.log("Player's hand: ", twentyOne.playersHand(player1));
​
// set randomization paramters on chain
vm.prevrandao(1);
vm.roll(1);
​
// expecting reverts without data because [OutOfFunds]
vm.expectRevert(bytes(""));
​
// Player calls to compare hands
vm.prank(player1);
twentyOne.call();
​
// expecting revert when restarting the game
vm.expectRevert(bytes("Player's deck is already initialized"));
​
// player tries to restart the game
vm.prank(player1);
twentyOne.startGame{value: 1 ether}();
}

Impact

The function TwentyOne::endGame reverts if there are insufficient funds in the contract to payout the winning player. Thus, players cannot receive payouts for winning a game unless they always loose before a win. In the case the function TwentyOne::endGame fails, the contract will be in a state where no new games can be started until the function TwentyOne::endGame is successfully called. This prevents the same player from playing until the contract is funded with the required amount of ETH or the function TwentyOne::call is repeately called until the player looses. This leads to a poor user experience and may prevent players from continuing to play the game. Due to confusion about not winning, players may also repeately call TwentyOne::call` until the function is successful which in this particular case means they loose the game.

Tools Used

Foundry, manual review

Recommendations

Add a check in the function TwentyOne::startGame to ensure that the contract balance is sufficient to pay out a winning player before starting a new game. The check should only allow a new game to start if the contract balance is sufficient to pay out all possible winning players. This will ensure that players can receive payouts for winning games and that the contract can continue to operate as expected.

Consider to add following code to the TwentyOne::startGame function:

function startGame() public payable returns (uint256) {
address player = msg.sender;
require(msg.value >= 1 ether, "not enough ether sent");
+ require(address(this).balance >= numberOfActivePlayers * 2 ether, "not enough funds to pay out winning player");
initializeDeck(player);
uint256 card1 = drawCard(player);
uint256 card2 = drawCard(player);
addCardForPlayer(player, card1);
addCardForPlayer(player, card2);
return playersHand(player);
}

To maintain protocol functionality for multiple cuncurrent players, consider following change in the TwentyOne::initializeDeck and TwentyOne::endGame function:

  • Change in TwentyOne::initializeDeck:

// Initialize the player's card pool when a new game starts
function initializeDeck(address player) internal {
require(availableCards[player].length == 0, "Player's deck is already initialized");
for (uint256 i = 1; i <= 52; i++) {
availableCards[player].push(i);
}
+ numberOfActivePlayers += 1;
}

  • Change TwentyOne::endGame:

// Ends the game, resets the state, and pays out if the player won
function endGame(address player, bool playerWon) internal {
delete playersDeck[player].playersCards; // Clear the player's cards
delete dealersDeck[player].dealersCards; // Clear the dealer's cards
delete availableCards[player]; // Reset the deck
if (playerWon) {
payable(player).transfer(2 ether); // Transfer the prize to the player
emit FeeWithdrawn(player, 2 ether); // Emit the prize withdrawal event
}
+ numberOfActivePlayers -= 1;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Insufficient balance for payouts / Lack of Contract Balance Check Before Starting Game

Support

FAQs

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