Description

The RockPaperScissors contract implements a commit-reveal pattern to prevent players from seeing each other's moves before making their own. However, the contract does not enforce any minimum entropy requirements for the salt values used in the commitment hashes. This allows players to use predictable or weak salt values, potentially making their moves guessable and undermining the security of the game.

Summary

The commit-reveal mechanism is designed to maintain fairness in the game by requiring players to first commit to a move by submitting a hash, then reveal the actual move and salt used to create that hash. The security of this pattern relies on the unpredictability of the salt value. Without requirements for salt entropy, players might use simple, guessable values such as bytes32(0), sequential numbers, or common strings like "salt", making their moves potentially predictable to sophisticated opponents.

Vulnerability Details

The contract never enforces or validates that the salt has sufficient entropy. It only checks that the hash of the move and salt matches the previously committed hash. This means a player could use a weak or predictable salt, potentially allowing an opponent to guess their move by brute-forcing common salt values.

Impact

1. Compromised game fairness: Players using weak salts may have their moves predicted, giving an unfair advantage to opponents.

2. Reduced security of commit-reveal: The core security mechanism becomes less effective for players who don't follow good salt generation practices.

3. Strategy leakage: In multi-turn games, a player who successfully guesses an opponent's move pattern based on weak salts gains a significant advantage.

While this vulnerability requires some sophistication to exploit and isn't guaranteed to work in all cases (depends on player behavior), it fundamentally undermines the commit-reveal security model that the game relies on.

Tools Used

Manual code review

Recommendations

1. Enforce minimum salt entropy:

   function commitMove(uint256 _gameId, bytes32 _commitHash, bytes32 _saltPreimage) external {

   // Verify salt has sufficient entropy

   require(uint256(_saltPreimage) > 1000000, "Salt too weak");

   // Rest of the function...

   }

2. Auto-incorporate additional entropy:

   function commitMove(uint256 _gameId, bytes32 _userSalt) external {

   // Add entropy automatically

   bytes32 strengthenedSalt = keccak256(abi.encodePacked(

   _userSalt,

   msg.sender,

   block.timestamp,

   blockhash(block.number - 1)

   ));

   bytes32 _commitHash = keccak256(abi.encodePacked(move, strengthenedSalt));

   // Store commitment...

   }

3. Document best practices: Provide clear guidelines to users on generating strong, unique salts for each game and move.