Summary
The playersHand, dealersHand, and initializeDeck functions in the contract use loops to process a potentially large number of cards. These loops could cause a Denial of Service (DoS) attack or excessive gas consumption under certain conditions. Specifically, if the number of cards in the player's or dealer's hand becomes large or if the deck initialization grows excessively (e.g., in edge cases or when the contract is being abused), it could cause the transaction to fail due to exceeding the block's gas limit.
playersHand(address player) function:
This function loops over all cards in a player's hand to calculate the total value of the hand. If a player accumulates a large number of cards (either due to game mechanics or an attack), the loop could consume excessive gas, potentially causing the transaction to revert due to hitting the block gas limit.
dealersHand(address player) function:
Similar to the playersHand function, this function loops over all cards in the dealer's hand to calculate the total value. If the dealer has a large number of cards, the loop could cause excessive gas consumption or failure to execute within the block's gas limit.
initializeDeck(address player) function:
The initializeDeck function initializes the deck by looping through 52 cards and adding them to the availableCards mapping. While 52 is a fixed number, if the number of players grows or if the function is called repeatedly without clearing or properly managing the deck, the number of cards to process could result in excessive gas consumption. Additionally, if a player is re-initialized multiple times or the deck is improperly reset, it could cause a loop to run too many times, leading to a DoS scenario.
These loops do not include checks to ensure that gas usage remains within acceptable limits, making the contract vulnerable to excessive gas consumption attacks.
Vulnerability Details
function testGasLimitOnExcessiveCards() public {
twentyOne = new TwentyOne();
address player = address(this);
vm.deal(player, 1 ether);
twentyOne.startGame{value: 1 ether}();
uint256 excessiveCards = 60;
for (uint256 i = 0; i < excessiveCards; i++) {
try twentyOne.hit() {
} catch Error(string memory reason) {
if (keccak256(bytes(reason)) == keccak256("Player is bust")) {
break;
}
}
}
uint256 gasBefore = gasleft();
uint256 playerTotal = twentyOne.playersHand(player);
uint256 gasAfter = gasleft();
uint256 gasUsed = gasBefore - gasAfter;
require(gasUsed < 10_000_000, "Gas usage exceeded reasonable limit");
}
Impact
The impact of these loops is significant because they could:
1. Cause Transaction Failures:
If the loops exceed the gas limit, transactions will revert, preventing players from interacting with the game.
2. Excessive Gas Consumption:
If the loops process a large number of cards, they could consume all available gas, blocking other functions or interactions with the contract.
3. Denial of Service (DoS) Attack:
Malicious users could attempt to accumulate a large number of cards or trigger repeated calls to initializeDeck, leading to excessive gas usage and causing the contract to become unresponsive or unusable.
Tools Used
MANUAL REVIEW
FOUNDRY
Recommendations
To prevent gas consumption issues caused by these loops, the following solutions should be implemented:
1. Limit the Number of Cards in Hand:
Implement a maximum card limit for players and dealers to avoid excessive loops in the playersHand and dealersHand functions. For instance, once a player or dealer reaches 21 cards, further draws should be restricted, and the game should either end or require the player to make decisions based on the current hand size.
<details>
<summary>Code</summary>
```javascript
uint256 constant MAX_CARDS = 21;
function playersHand(address player) public view returns (uint256) {
uint256 playerTotal = 0;
require(playersDeck[player].playersCards.length <= MAX_CARDS, "Too many cards in hand");
for (uint256 i = 0; i < playersDeck[player].playersCards.length; i++) {
uint256 cardValue = playersDeck[player].playersCards[i] % 13;
playerTotal += (cardValue >= 10) ? 10 : cardValue;
}
return playerTotal;
}```
</details>
2. Avoid Repeated Initialization of Decks:
Implement checks in the initializeDeck function to ensure that the deck is only initialized once per player, and add proper handling to avoid re-initialization that leads to excessive gas usage.
<details>
<summary>Code</summary>
```javascript
function initializeDeck(address player) internal {
require(availableCards[player].length == 0, "Player's deck is already initialized");
}```
</details>
3. Limit Deck Size or Split Decks for Multiple Players:
Consider dividing the card deck into smaller portions or limiting the number of players to avoid running excessive loops over large sets of cards. If the game allows multiple players, consider initializing separate decks per player rather than sharing one large deck.
4. Check for Gas Usage Before Calling Loops
For functions that rely on loops (like playersHand and dealersHand), implement checks to ensure that the number of cards does not exceed a reasonable threshold to prevent excessive gas consumption.
5. Gas Limit Testing:
Conduct thorough gas usage tests to ensure that the contract remains efficient under various scenarios, especially with large numbers of cards and players.