TwentyOne

First Flight #29
Beginner FriendlyGameFiFoundrySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Improper Handling of Ace Cards

Summary
In the TwentyOne.sol Blackjack smart contract, the handling of Ace cards (which can be worth either 1 or 11 points) is not properly implemented in both the player's and the dealer's hand calculations. This issue leads to incorrect game logic, potentially resulting in faulty game outcomes. The logic for the dealer's hand also lacked proper handling for the Ace, which may cause inconsistent game results.

Vulnerability Details

The vulnerability lies in the improper handling of Ace cards, a crucial aspect of Blackjack game rules. In the original code:

  • For the player’s hand (playersHand function): Aces are treated as 10, which is incorrect. Aces should be treated as 1 initially, and if the total hand value allows, an Ace should be counted as 11.

  • For the dealer’s hand (dealersHand function): Aces are not properly accounted for in the hand's total value, and the dealer’s hand calculation logic is inconsistent with the player’s hand calculation.

The issue can cause incorrect total calculations for both the player and the dealer, affecting the outcome of the game. A player might think they have a valid total, but the game may incorrectly register their hand, leading to undesired consequences.

Code Fragment from the Original Implementation:

Player's Hand Logic:

if (cardValue == 0 || cardValue >= 10) {
playerTotal += 10;
}

Dealer's Hand Logic:

if (cardValue >= 10) {
dealerTotal += 10;
}

Problems:

  1. Player's hand: Aces are treated as 10, which can mislead the player into thinking their hand is better or worse than it really is.

  2. Dealer's hand: Aces are not handled at all. The dealer’s total could be incorrectly calculated, which could affect the final outcome of the game.

Impact

  • Incorrect Card Values: Aces are treated incorrectly, potentially causing incorrect totals for both the player and the dealer.

  • Faulty Game Outcomes: The mismatch in hand totals can lead to an unfair game where either the player or the dealer might win or lose incorrectly.

  • Player Experience: The player might receive incorrect feedback (such as winning or losing), as their hand total is not calculated correctly.

  • Smart Contract Integrity: If not addressed, this flaw could undermine the trust in the smart contract, leading to disputes or manipulation of the game logic.

Tools Used


Manual Code Review:
The code was manually reviewed for logical errors related to the handling of card values, especially Aces.

Recommendations

The issue can be resolved by fixing the Ace-handling logic in both the playersHand and dealersHand functions. This will ensure that Aces are treated as either 1 or 11 depending on the context, thus adhering to the correct rules of Blackjack.

Code Fix for playersHand:

function playersHand(address player) public view returns (uint256) {
uint256 playerTotal = 0;
uint256 acesCount = 0; // To track how many aces are present
for (uint256 i = 0; i < playersDeck[player].playersCards.length; i++) {
uint256 cardValue = playersDeck[player].playersCards[i] % 13;
if (cardValue == 0) {
playerTotal += 1; // Treat Ace as 1 initially
acesCount++; // Count the Ace
} else if (cardValue >= 10) {
playerTotal += 10; // Face cards (Jack, Queen, King) count as 10
} else {
playerTotal += cardValue; // Other cards are their face value
}
}
// If there are Aces, and the total is 11 or less, add 10 to the total (Ace as 11)
if (acesCount > 0 && playerTotal <= 11) {
playerTotal += 10;
}
return playerTotal;
}

Code Fix for dealersHand:

function dealersHand(address player) public view returns (uint256) {
uint256 dealerTotal = 0;
uint256 acesCount = 0; // To track how many aces are present
for (uint256 i = 0; i < dealersDeck[player].dealersCards.length; i++) {
uint256 cardValue = dealersDeck[player].dealersCards[i] % 13;
if (cardValue == 0) {
dealerTotal += 1; // Treat Ace as 1 initially
acesCount++; // Count the Ace
} else if (cardValue >= 10) {
dealerTotal += 10; // Face cards (Jack, Queen, King) count as 10
} else {
dealerTotal += cardValue; // Other cards are their face value
}
}
// If there are Aces, and the total is 11 or less, add 10 to the total (Ace as 11)
if (acesCount > 0 && dealerTotal <= 11) {
dealerTotal += 10;
}
return dealerTotal;
}
Updates

Lead Judging Commences

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

Wrong ace value

Asymmetric calculation of hands is rigged in the player`s favor.

Support

FAQs

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