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

Player wrongly wins the game

0xgondar

Summary

Due to a mishandling of else if statement in the TwentyOne::call function, the player wins the game even if they should not.

Vulnerability Details

In TwentyOne::call function, in order to determine the winner the following logic is used:

  • if dealer's hand is over 21 => player wins

  • else if player's hand is greater than the dealer's => player wins (here's the bug)

  • else, player loses

The problem with this logic is that in that else if statement, player draws another card, and thus, in that draw, player's hand can go over 21, leading to a loss.

Here's the function in question

function call() public {
require(playersDeck[msg.sender].playersCards.length > 0, "Game not started");
uint256 playerHand = playersHand(msg.sender);
​
// Calculate the dealer's threshold for stopping (between 17 and 21)
uint256 standThreshold =
(uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender, block.prevrandao))) % 5) + 17;
​
// Dealer draws cards until their hand reaches or exceeds the threshold
while (dealersHand(msg.sender) < standThreshold) {
uint256 newCard = drawCard(msg.sender);
addCardForDealer(msg.sender, newCard);
}
​
uint256 dealerHand = dealersHand(msg.sender);
​
// Determine the winner
if (dealerHand > 21) {
emit PlayerWonTheGame("Dealer went bust, players winning hand: ", playerHand);
endGame(msg.sender, true);
} else if (playerHand > dealerHand) {
//@audit should be playerhand <= 21 && playerhand > dealerhand
emit PlayerWonTheGame("Dealer's hand is lower, players winning hand: ", playerHand);
endGame(msg.sender, true);
} else {
emit PlayerLostTheGame("Dealer's hand is higher, dealers winning hand: ", dealerHand);
endGame(msg.sender, false);
}
}

There's another bug relating to the dealer being called as msg.sender (same as the player, but I submitted that bug in another issue).

Impact

Player wins when they should not.

Tools Used

Manual review

Recommendations

Change else if (playerHand > dealerHand) to else if (playerHand <= 21 && playerHand > dealerHand)

Updates

Lead Judging Commences

inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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