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

User Can Deposit More Than 1 Ether But Payout Is Limited to 2 Ether Leading to Potential Loss of Funds.

pollersan

Summary

The project documentation specifies that the user should deposit 1 ether, as the maximum possible profit is 2 ether. However, the startGame function allows deposits greater than 1 ether, which could result in a loss of user funds since the maximum payout is still capped at 2 ether.

Vulnerability Details

The issue is in the line require(msg.value >= 1 ether, "not enough ether sent");, which enforces a minimum deposit of 1 ether but does not impose a maximum limit. As a result, the user can deposit more than 1 ether. However, regardless of the deposit amount, the user can only receive a maximum of 2 ether, leading to a potential loss of funds for deposits exceeding this limit.

function startGame() public payable returns (uint256) {
address player = msg.sender;
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); // not bad
}

Impact

If a user deposits more than 1 ether, believing they will always win double their deposit as in blackjack, they will lose the extra funds sent. These funds cannot be recovered, as there is no way to withdraw the deposit. The user will only receive 2 ether upon winning.

Example:


A user reads in the documentation that they will win double their deposit, so they deposit 5 ether. After winning, they only receive 2 ether instead of 10 ether. Instead of gaining 5 ether, they have lost 3 ether.

Proof of Concept

Add this code to the test file and run it to observe the vulnerability. You will see that you can deposit more than 1 ether.

function testPlayerCanDepositMoreThanOneEther() public {
​
uint256 initialBalanceContract = address(twentyOne).balance;
vm.startPrank(player1);
​
twentyOne.startGame{value: 5 ether}();
​
vm.stopPrank();
​
uint256 finalBalanceContract = address(twentyOne).balance;
console.log("Initial contract balance:", initialBalanceContract);
console.log("Final contract balance:", finalBalanceContract);
​
assertEq(finalBalanceContract, initialBalanceContract + 5 ether);
​
}

Tools Used

Manual Review.

Recommendations

Changing require(msg.value >= 1 ether, "not enough ether sent"); to require(msg.value == 1 ether, "not enough ether sent"); will mitigate the issue by ensuring that only exactly 1 ether can be sent. This prevents users from accidentally sending more funds that could be lost, as the deposit limit will be strictly enforced.

Corrected Code Based on the Recommendation:

function startGame() public payable returns (uint256) {
address player = msg.sender;
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 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] User mistake, too much ETH sent

Support

FAQs

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