Rock Paper Scissors

Summary

A vulnerability exists in the Rock-Paper-Scissors game smart contract's move verification mechanism. There's a type inconsistency between how move hashes are likely generated off-chain during commitment and how they're verified on-chain during the reveal phase. This inconsistency could lead to valid move reveals being rejected, causing players to lose games due to technical issues rather than gameplay.

Vulnerability Details

The vulnerability is in the revealMove function, specifically in how the commitment hash is recalculated for verification:

The issue arises because:

1. The function accepts a uint8 _move parameter from the user

2. It converts this to a Move enum type with Move move = Move(_move)

3. It then uses the enum value in the hash calculation: keccak256(abi.encodePacked(move, _salt))

However, when players initially create their commitment hash off-chain (before calling commitMove), they would likely use numerical values (1, 2, or 3) in their hash calculation (although this was not specified), not the enum representation that exists in the contract.

The abi.encodePacked() of an enum value might not produce the same binary representation as the abi.encodePacked() of its underlying integer value, especially across different environments and implementations.

Impact

• Players who correctly committed moves may have their reveals rejected because the on-chain hash calculation differs from their off-chain calculation.

• Players could lose games due to timeout penalties when their legitimate reveals fail.

Tools Used

Manual code review

Recommendations

Modify the hash calculation in the revealMove function to use the raw uint8 value rather than the enum:

Alternatively, you can ensure that both calculations on and off chain and done in the same manner, ensuring consisitency.