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

Inconsistent modulus operation in `TwentyOne::dealerHand`

web3mio

Summary

If the game is modeled the Twenty, both player & dealer should have the same condition. However, when cardValue == 0 for the dealer, it doesn't return the value 10.

Vulnerability Details

Cards are computed as cardValue = card % 13, which can result in 0 for certain cards (13, 26, 39, 52).

When the first call & second call for dealerHand have values of cardValue returns zero, dealerTotal will be zero, which is unfair for players.

POC

Add this test to the TwentyOne.t.sol file.

​
function testInconsistent_DealerCardValue() public {
vm.startPrank(player1); // Start acting as player1
twentyOne.startGame{value: 1 ether}();
​
// Mock a scenario where dealer's card value includes 0
uint256 dealerCardValue = 0; // Value to test
vm.mockCall(
address(twentyOne),
abi.encodeWithSignature("dealersHand(address)", player1),
abi.encode(dealerCardValue) // Mock dealersHand to return 0
);
​
​
// Call dealersHand and check the response
uint256 dealerTotal = twentyOne.dealersHand(player1);
​
// Ensure dealerTotal matches expected behavior
assertEq(dealerTotal, dealerCardValue, "Dealer's total should handle cardValue == 0 correctly");
​
vm.stopPrank();
}
​

Run forge test --match-test testInconsistent_DealerCardValue -vvv on your terminal.

Result:

Ran 1 test for test/TwentyOne.t.sol:TwentyOneTest
[PASS] testInconsistent_DealerCardValue() (gas: 1250725)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 13.35ms (12.09ms CPU time)
​
Ran 1 test suite in 25.98ms (13.35ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

  • With this design choice, the Player will have a higher chance of losing to the dealer as cardValue == 0 will lower the chance of a high dealerTotal.

  • The cardValue == 0 is not an ideal model for the game of TwentyOne.

Tools Used

Foundry 0.2.0 and the console functionality.

Recommendations

Replace the TwentyOne::dealersHand with the following.

function dealersHand(address player) public view returns (uint256) {
uint256 dealerTotal = 0;
for (uint256 i = 0; i < dealersDeck[player].dealersCards.length; i++) {
uint256 cardValue = dealersDeck[player].dealersCards[i] % 13;
+ if (cardValue == 0 || cardValue >= 10) {
+ dealerTotal += 10;
+ } else {
dealerTotal += cardValue;
}
}
return dealerTotal;
}
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.