TwentyOne

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

[H-4] TwentyOne::dealersHand calculcation is off by one for cards from 2 through 10 inclusive

Summary

In case a cardValue of 0 is considered an Ace - The TwentyOne::dealersHand calculation wrongly adds the cardValue for the equivalent of the real world cards 2,3,4,5,6,7,8,9,10 because, the cardValue to add calculation, does modulo 13 division, of the numbers 1 through 52, which are supposed to represent a full card deck, and adds the result to the TwentyOne::dealersHand::dealerTotal. This is problematic because it does not account for real world cards not having a 1 value, while the modulo 13 division produces the result 1. This effectively results in returned TwentyOne::dealersHand::dealerTotal being one less than it should be, for each drawn card that is between 2,3,4,5,6,7,8,9,10.

Vulnerability Details

The TwentyOne::dealersHand calculation wrongly adds the following cardValue:

uint256 cardValue = dealersDeck[player].dealersCards[i] % 13;

to the TwentyOne::dealersHand::dealerTotal in the cases where the cardValue ends up between 1 and 10, due to "else" in this statement:

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

Issue

  • Card values in the range [1, 9] (representing real-world cards 2 through 10) are incorrectly added to dealerTotal as cardValue.

  • The correct implementation should add these card values as cardValue + 1.

  • Card values in the range [10, 12] should be added as 10, representing the equivalent of the real-world cards Jack (J), Queen (Q), and King (K).

  • Aces (value 0 in modulo 13) should be handled separately according to BlackJack rules. Refer to my H-5 finding below for more details.

Correct Behavior

  • For card values 1 through 9: dealerTotal += cardValue + 1;

  • For card values 10 through 12: dealerTotal += 10;

  • Aces (value 0): Handle separately based on [H-5] finding.

Impact

This bug occurs 100% of the time and causes incorrect calculations for TwentyOne::dealersHand. This can result in:

  1. Players losing when they should have won.

  2. Players winning when they should have lost.

This undermines the fairness and reliability of the game.

Tools Used

  1. Manual Review

  2. Foundry (PoC unit tests can be provided upon request)

  3. ChatGPT

Recommendations

  1. Rewrite the if-else Statements:
    Update the if-else logic to properly calculate the TwentyOne::dealersHand::dealerTotal in line with BlackJack rules. The correct implementation should follow these rules:

    • Card values in the range [1, 9] (representing real-world cards 2 through 10) should be added as cardValue + 1.

    • Card values in the range [10, 12] (representing J, Q, K) should be added as 10.

    • Aces (value 0) should be handled separately. Refer to my H-5 finding

Updates

Lead Judging Commences

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

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.