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

Not Enough Funds to Transfer to Winner will Revert with Funds Stuck in The Contract

jjsonchain

Summary

The docs and code suggest that if a player has beaten the dealer, and therefore won the game, they will be rewarded with 2 ETH. However, there is a high chance that the transfer will revert for lack of funds.

Vulnerability Details

The endGame function resets the player's decks and transfers them a reward of 2 ETH:

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
}
}

However, there is no pool or initial transfer to the contract to make sure there are enough funds to reward the winners.

Impact

The protocol will have locked funds in the contract, from the players that deposited 1 ETH to start playing, but cannot get rewarded. Lets consider the following scenario:

  1. Bob start game, by depositing 1 ETH.

  2. Bob plays the game and beats the dealer.

  3. There is only 1 ETH in the contract.

  4. The endGame function tries to transfer 2 ETH but it has only 1 ETH.

  5. Function reverts and player lost their 1 ETH.

The test in the test folder also fails:

function test_Call_PlayerWins() public {
vm.startPrank(player1); // Start acting as player1
​
twentyOne.startGame{value: 1 ether}();
​
// Mock the dealer's behavior to ensure player wins
// Simulate dealer cards by manipulating state
vm.mockCall(
address(twentyOne),
abi.encodeWithSignature("dealersHand(address)", player1),
abi.encode(18) // Dealer's hand total is 18
);
​
uint256 initialPlayerBalance = player1.balance;
​
// Player calls to compare hands
twentyOne.call();
​
// Check if the player's balance increased (prize payout)
uint256 finalPlayerBalance = player1.balance;
assertGt(finalPlayerBalance, initialPlayerBalance);
​
vm.stopPrank();
}

Tools Used

Foundry Unit Tests

Recommendations

The way to fix it can be a couple of different options, however the most important part is to have an invariant that makes sure that the amount to be awarded cannot be more than the total funds locked in the contract:

+ uint256 totalFunds;
​
function startGame() public payable returns (uint256) {
address player = msg.sender;
require(msg.value >= 1 ether, "not enough ether sent");
totalFunds += msg.value;
uint256 card1 = drawCard(player);
uint256 card2 = drawCard(player);
addCardForPlayer(player, card1);
addCardForPlayer(player, card2);
return playersHand(player);
}
​
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) {
+ require(totalFunds > 2, "Not enough funds in contract")
+ totalFunds -= 2;
payable(player).transfer(2 ether); // Transfer the prize to the player
emit FeeWithdrawn(player, 2 ether); // Emit the prize withdrawal event
}
}
​
function getPlayerCards(address player) public view returns (uint256[] memory) {
return playersDeck[player].playersCards;
}
​
function getDealerCards(address player) public view returns (uint256[] memory) {
return dealersDeck[player].dealersCards;
}

Consider a few options:

  • Adding a liquidity pool.

  • Having an initial deposit into the contract.

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.