TwentyOne

Summary

The contract uses a push-based withdrawal mechanism to transfer winnings to players, which creates a Denial of Service (DoS) vulnerability. If the contract balance is insufficient at the time of payout, the transfer call reverts, and the player is unable to receive their winnings. Furthermore, there is no mechanism for players to retry withdrawing their winnings later, even if the contract is subsequently funded. This results in permanent loss of winnings for affected players and disrupts the game flow.

Vulnerability Details

1. Root Cause:

   • The endGame function transfers winnings to the player directly using the transfer method:

     if (playerWon) {

     payable(player).transfer(2 ether);

     emit FeeWithdrawn(player, 2 ether);

     }

   • This push-based method:

     • Relies on the contract’s balance being sufficient at the time of execution.

     • Does not provide a fallback mechanism to allow players to withdraw their winnings later.

2. Symptoms:

   • If the contract balance is insufficient when a player wins:

     • The transfer call fails, causing the entire transaction to revert.

     • The player is unable to retry withdrawing their winnings, even if the contract is subsequently funded.

3. Code Affected:

   • endGame Function:

     if (playerWon) {

     payable(player).transfer(2 ether);

     emit FeeWithdrawn(player, 2 ether);

     }

     ​

4. Behavior Details:

   • Players’ winnings are tied to the immediate state of the contract balance. If funds are insufficient, the winnings are permanently inaccessible.

Impact

• Permanent Loss of Winnings:

  • Players cannot retrieve their winnings if the prize payout fails due to insufficient contract balance.

• Denial of Service for Prize Withdrawals:

  • The push-based mechanism locks players out of their funds with no option to retry later, disrupting the game flow.

• Reputation Risk:

  • Players may lose trust in the protocol if winnings are not consistently and reliably paid out.

Tools Used

Manual code review.

Recommendations

1. Switch to Pull-Based Withdrawal Mechanism:
   Replace the push-based prize withdrawal mechanism with a pull-based approach, where players can manually withdraw their winnings when the contract balance is sufficient. This prevents reverts due to insufficient funds at the time of payout and provides a fallback for players to claim their prizes later.

   Implementation:

   • Modify the endGame function to record the player’s winnings instead of transferring them immediately:

     mapping(address => uint256) public winnings;

     ​

     if (playerWon) {

     winnings[player] += 2 ether;

     emit FeeWithdrawn(player, 2 ether);

     }

   • Add a withdrawWinnings function that allows players to claim their winnings when the contract balance is sufficient:

     function withdrawWinnings() public {

     uint256 amount = winnings[msg.sender];

     require(amount > 0, "No winnings to withdraw");

     require(address(this).balance >= amount, "Not enough funds in contract");

     ​

     winnings[msg.sender] = 0; // Reset before transfer to avoid reentrancy

     payable(msg.sender).transfer(amount);

     }