Summary
TwentyOne::call function has a logic error and the player wins even with playersHand > 21.
TwentyOne::call function has a logic error and doesn't check for if the playersHand > 21 and players wins even if (playersHand > 21 when playersHand > dealersHand)
Vulnerability Details
In TwentyOne::call function
if (dealerHand > 21) {
emit PlayerWonTheGame(
"Dealer went bust, players winning hand: ",
playerHand
);
endGame(msg.sender, true);
-->> } else if (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);
}
Impact
The player will win even if his score is greater than 21, which should not happen and will break tha game.
Proof of Code
Run this test by making TwentyOne::addCardForPlayer , TwentyOne::addCardForDealer , TwentyOne::initializeDeck functions public and commenting out the standThreshold of dealer in the TwentyOne::call function
function testCallPlayerHandGreaterThan21AndDealer() public {
vm.startPrank(player1);
vm.deal(address(twentyOne), 10 ether);
twentyOne.initializeDeck(player1);
twentyOne.addCardForPlayer(player1, 10);
twentyOne.addCardForPlayer(player1, 10);
twentyOne.addCardForPlayer(player1, 10);
twentyOne.addCardForDealer(player1, 12);
twentyOne.addCardForDealer(player1, 9);
uint256 playerHand = twentyOne.playersHand(player1);
uint256 dealerHand = twentyOne.dealersHand(player1);
assertGt(playerHand, 21, "Player's hand is not greater than 21");
assertGt(playerHand, dealerHand, "Player's hand is not greater than dealer's hand");
vm.expectEmit(true , true , true , true);
emit TwentyOne.PlayerWonTheGame("Dealer's hand is lower, players winning hand: ",
playerHand);
twentyOne.call();
vm.stopPrank();
}
Tools Used
Manual Review
Recommendations
Add the following in your TwentyOne::call function
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 && playerHand <= 21>) {
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);
}
}
