TwentyOne

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

Failed Transfers in call() Function Can Lead to Lost Winnings

Summary

The TwentyOne contract directly transfers ETH to winners in the call() function. If this transfer fails, the entire transaction reverts, forcing players to call again with different random values that might result in a loss. The contract should separate game resolution from payment distribution.

Vulnerability Details

  • Unsafe Direct Transfer:

    • Game result and ETH transfer are coupled in the same transaction

    • Transfer failure reverts the entire game result

    • Players must replay hands with new randomness if transfer fails

function endGame(address player, bool playerWon) internal {
// ... game state cleanup ...
if (playerWon) {
payable(player).transfer(2 ether); // Unsafe direct transfer
}
}

Impact

  • Players can lose legitimate winnings if transfer fails

  • Forced replay of winning hands can turn into losses

  • Contract does not maintain record of owed payouts

  • Players might lose trust if winnings aren't received

Tools Used

  • Manual Code Review

Recommendations

  • Implement a withdrawal pattern:

mapping(address => uint256) public playerBalances;
function endGame(address player, bool playerWon) internal {
// ... game state cleanup ...
if (playerWon) {
playerBalances[player] += 2 ether;
}
}
function withdraw() external {
uint256 amount = playerBalances[msg.sender];
require(amount > 0, "No balance");
playerBalances[msg.sender] = 0;
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Transfer failed");
}
Updates

Lead Judging Commences

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

Revert a bad outcome

Support

FAQs

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