Rock Paper Scissors

Summary

The commitMove() function in the Rock Paper Scissors contract allows players to submit an arbitrary bytes32 value as their move commitment (_commitHash) without any verification. This critical design flaw renders the commit-reveal mechanism non-functional, as the contract cannot reliably verify if the revealed move corresponds to the committed hash, preventing games from progressing.

Vulnerability Details

Affected Functions

Technical Analysis

1. Unvalidated Commitment: The commitMove() function accepts an arbitrary bytes32 _commitHash provided by the player and directly stores it as game.commitA or game.commitB.

2. Mismatched Verification: The revealMove() function attempts to verify the revealed move by hashing it with the provided salt and comparing it to the stored commitment. However, since the stored commitment could be any arbitrary value, the calculated hash will almost never match, preventing successful reveals.

3. Broken Logic: The core logic of the commit-reveal scheme relies on the commitment being cryptographically tied to the actual move and salt. This link is absent in the current implementation.

Attack Scenario

Actors:

• Alice: Player A

• Contract: Vulnerable Rock Paper Scissors game instance

Steps:

1. Alice's Commit: Alice calls commitMove() with an arbitrary _commitHash (e.g., 0x123). The contract stores game.commitA = 0x123.

2. Alice's Reveal Attempt: Alice calls revealMove() with her actual move (e.g., Rock - 1) and a salt (e.g., "alice_salt"). The contract calculates keccak256(abi.encodePacked(1, "alice_salt")), which will almost certainly not be equal to 0x123. The require(commit == game.commitA, ...) check will fail.

Impact

• The commit-reveal mechanism is broken. Players will almost never be able to successfully reveal their moves.

• Games will likely get stuck in the commit phase

Tools Used

• Manual Code Review: Identified the logical flaw in the commitMove() function.

Recommendations

The commitMove() function must be modified to ensure that the stored commitment is cryptographically linked to the player's actual move and a secret salt. The function should either:

1. Accept the move and salt in commitMove() and generate the hash internally.

2. Require the player to submit the hash of their move and salt as the commitment.