TwentyOne

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

Incorrect use of msg.sender led to the player being mistakenly added to the hand

Summary

Incorrect use of msg.sender led to the player's hand being added again, causing an error in the stand calculation.

Vulnerability Details

https://github.com/Cyfrin/2024-11-TwentyOne/blob/a4429168302722d14a5e5996d25d6fc5be22a899/src/TwentyOne.sol#L128-L140
As we can see above, msg.sender is used, but this call function is invoked by the player. This means that at this point, msg.sender is the player, so using it to perform actions for the dealer is a logical error

Impact

  • The standThreshold will be set incorrectly.

  • uint256 newCard = drawCard(msg.sender); — The player is mistakenly added a card.

  • uint256 dealerHand = dealersHand(msg.sender); — At this point, the dealer's hand is incorrectly calculated as the player's hand value.

Tools Used

manual inspection

Recommendations

  • Add a function dealerTurn(dealer) to automatically handle the dealer's actions, separating the logic for the player's call and the dealer's card drawing, ensuring that msg.sender is not misused.

function dealerTurn(address dealer) internal {
// Calculate the dealer's threshold for stopping (between 17 and 21)
uint256 standThreshold = (uint256(
keccak256(
abi.encodePacked(block.timestamp, dealer, block.prevrandao)
)
) % 5) + 17;
// Dealer draws cards until their hand reaches or exceeds the threshold
while (dealersHand(dealer) < standThreshold) {
uint256 newCard = drawCard(dealer);
addCardForDealer(dealer, newCard);
}
}
function call() public {
require(
playersDeck[msg.sender].playersCards.length > 0,
"Game not started"
);
uint256 playerHand = playersHand(msg.sender);
// Call the function to automatically draw cards for the dealer here.
dealerTurn(dealer);
uint256 dealerHand = dealersHand(dealer);
// Determine the winner
//....
}
}
  • Directly use the dealer instead of msg.sender

uint256 standThreshold = (uint256(
keccak256(
abi.encodePacked(block.timestamp, dealer, block.prevrandao)
)
) % 5) + 17;
// Dealer draws cards until their hand reaches or exceeds the threshold
while (dealersHand(dealer) < standThreshold) {
uint256 newCard = drawCard(dealer);
addCardForDealer(dealer, newCard);
}
uint256 dealerHand = dealersHand(dealer);
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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