TwentyOne

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

The requirement of `msg.value` in `TwentyOne::startGame` should be strict equal to 1 ether, since the contract lacks a refund logic when extra ETH is sent.

Description

The TwentyOne::startGame function in L::95 allows the players to send more than 1 ether to start the game, but the contract doesn't implement a refund process when extra ETH is sent.

Impact

Players by confusion or mistake may send more than 1 ETH when calling the TwentyOne::startGame function, causing the loss of this extra ETH since the contract doesn't have a refund logic. This would happen either if the player wins or losses the game, the excess ETH is never sent back.

Proof of Concepts

Add the following test in TwentyOne.t.sol.

function test_ExcessETHSentIsLost() public {
vm.startPrank(player1); // Start acting as player1
twentyOne.startGame{value: 2 ether}();
// Mock the dealer's behavior to ensure player wins
// Simulate dealer cards by manipulating state
vm.mockCall(
address(twentyOne),
abi.encodeWithSignature("dealersHand(address)", player1),
abi.encode(22) // Dealer's hand total is 18
);
// Balance is equal to 10 ether due to the vm.deal in the setUp()
uint256 initialPlayerBalance = 10 ether;
// Player calls to compare hands
twentyOne.call();
// The balance difference should be zero since it won 1 ether, but didn't get the refund of the excess sent
uint256 finalPlayerBalance = player1.balance;
assertEq(0, finalPlayerBalance - initialPlayerBalance);
vm.stopPrank();
}

Recommended mitigation

Restrict the msg.value to be equal to 1 ether or add a refund logic inside the TwentyOne::engGame function once the player had won or lost.

function startGame() public payable returns (uint256) {
address player = msg.sender;
- require(msg.value >= 1 ether, "not enough ether sent");
+ require(msg.value == 1 ether, "not enough ether sent");
initializeDeck(player);
uint256 card1 = drawCard(player);
uint256 card2 = drawCard(player);
addCardForPlayer(player, card1);
addCardForPlayer(player, card2);
return playersHand(player);
}
Updates

Lead Judging Commences

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

Insufficient balance for payouts / Lack of Contract Balance Check Before Starting Game

Support

FAQs

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