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

No limit on game creation in `RockPaperScissors` contract can lead to permanent storage bloat

s3bc40

Summary

Games are created without any limit. Which means, one user could create an unlimited number of games. With winning tokens, he could create a game for each token, for free. The struct Game, which takes up 11 slots, is stored in the contract storage without any cleanup or deletion mechanism. So, if we keep adding entries to the games mapping and never remove old/finished ones, your contract's state will keep growing forever.

Vulnerability Details

Impact

In a long term, this could lead to bloat in the contract storage, which could make it impossible to interact with. There is a risk of Denial of Service (DoS) if the contract storage becomes too large. This could lead to a situation where users are unable to create or join games, effectively locking them out of the contract.

POC

This is how the storage of the RockPaperScissors contract looks like:

### Storage BEFORE creating games ###
Game counter: 0
Game ID 0
Slot 0 = 0x0000000000000000000000000000000000000000
Slot 1 = 0x0000000000000000000000000000000000000000
Slot 2 = 0
Slot 3 = 0
Slot 4 = 0
Slot 5 = 0
Slot 6 = 0
Slot 7 = 0
Slot 8 = 0
Slot 9 =
0x0000000000000000000000000000000000000000000000000000000000000000
Slot 10 =
0x0000000000000000000000000000000000000000000000000000000000000000
Slot 11 =
moveA 0
moveB 0
scoreA 0
scoreB 0
state 0

The Game struct takes up 11 slots, and the games mapping is public. This means that every time a game is created, the contract storage grows by 11 slots. If a user creates an unlimited number of games, the contract storage will grow indefinitely.

### Storage AFTER creating games ###
Game counter: 100
Game ID 99
Slot 0 = 0x23223AC37AC99a1eC831d3B096dFE9ba061571CF
Slot 1 = 0x0000000000000000000000000000000000000000
Slot 2 = 0
Slot 3 = 600
Slot 4 = 0
Slot 5 = 1
Slot 6 = 86401
Slot 7 = 3
Slot 8 = 1
Slot 9 =
0x0000000000000000000000000000000000000000000000000000000000000000
Slot 10 =
0x0000000000000000000000000000000000000000000000000000000000000000
Slot 11 =
moveA 0
moveB 0
scoreA 0
scoreB 0
state 0

After creating 100 games, the contract storage has grown by 1100 slots. And at the end of each game (or cancelled), the game is not removed from the storage.

### Storage AFTER finishing a game ###
Game counter: 100
Game ID 99
Slot 0 = 0x23223AC37AC99a1eC831d3B096dFE9ba061571CF
Slot 1 = 0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1
Slot 2 = 0
Slot 3 = 600
Slot 4 = 601
Slot 5 = 1
Slot 6 = 86401
Slot 7 = 3
Slot 8 = 3
Slot 9 =
0x8fe307739b80c82d28eabac96094c91e2a3ec596189134a788848aad38618915
Slot 10 =
0x97c8d8f22b617f32f21d4f643a756649d13ff7fa3bf0166f78f9f8cc768540bc
Slot 11 =
moveA 3
moveB 2
scoreA 3
scoreB 0
state 3

Imagine if a user creates 1000 games, the contract storage will grow by 11000 slots. And if the user never removes the games, the contract storage will keep growing indefinitely.

Tools Used

Foundry test:

/**
* @dev This test checks the storage bloat of the game contract when creating multiple games.
* It creates 100 games and then plays a game to see if the storage layout changes.
* It also prints the storage layout before and after creating the games.
* This is useful for auditing purposes to ensure that the contract is not using excessive storage.
* https://solodit.cyfrin.io/issues/m-02-an-attacker-can-bloat-the-pink-runtime-storage-with-zero-costs-code4rena-phala-network-phala-network-git
*/
function testAuditCreateMultipleGameStorageBloat() public {
// Arrange
vm.prank(address(game));
token.mint(playerA, 100);
vm.stopPrank();
​
// Log storage of first few games BEFORE creation
console2.log("### Storage BEFORE creating games ###");
_printGameMappingStorageLayout();
​
// Act
// If playerA creates 100 games
vm.startPrank(playerA);
for (uint256 i = 0; i < 100; i++) {
token.approve(address(game), 1);
gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);
}
​
// Log storage of first few games AFTER creation
console2.log("### Storage AFTER creating games ###");
_printGameMappingStorageLayout();
​
// Continue gameplay as you originally had
vm.startPrank(playerB);
token.approve(address(game), 1);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithToken(gameId);
vm.stopPrank();
​
(, , , , , , , uint256 totalTurns, , , , , , , , ) = game.games(gameId);
​
for (uint256 i = 0; i < totalTurns; i++) {
playTurn(
gameId,
RockPaperScissors.Move.Scissors,
RockPaperScissors.Move.Paper
);
​
console2.log("### Storage AFTER game turn", i + 1, "###");
_printGameMappingStorageLayout();
}
​
console2.log("### Storage AFTER finishing a game ###");
_printGameMappingStorageLayout();
​
assertTrue(true);
}
​
/**
* @dev Prints the storage layout of the game mapping.
* This is for debugging purposes and should not be used in production.
*/
function _printGameMappingStorageLayout() internal view {
console2.log("Game counter: ", game.gameCounter());
bytes32 base = keccak256(abi.encode(gameId, uint256(0)));
console2.log("Game ID", uint256(gameId));
for (uint256 j = 0; j < 12; j++) {
bytes32 slot = bytes32(uint256(base) + j);
bytes32 val = vm.load(address(game), slot);
​
if (j == 0 || j == 1) {
address addr = address(uint160(uint256(val)));
console2.log("Slot", j, "=", addr);
} else if (j == 9 || j == 10) {
console2.log("Slot", j, "=");
console2.logBytes32(val);
} else if (j == 11) {
// Solidity packs uint8, enum, and similar small types together into a single slot when possible, to save space.
// That’s 5 bytes total, and Solidity packs all of that into a single slot (Slot 11).
console2.log("Slot", j, "=");
uint8 moveA = uint8(uint256(val) >> (8 * 0));
uint8 moveB = uint8(uint256(val) >> (8 * 1));
uint8 scoreA = uint8(uint256(val) >> (8 * 2));
uint8 scoreB = uint8(uint256(val) >> (8 * 3));
uint8 state = uint8(uint256(val) >> (8 * 4));
​
console2.log("moveA", moveA);
console2.log("moveB", moveB);
console2.log("scoreA", scoreA);
console2.log("scoreB", scoreB);
console2.log("state", state);
} else {
console2.log("Slot", j, "=", uint256(val));
}
}
}

Recommendations

Consider implementing a cleanup mechanism to remove old or finished games from the contract storage. This could be done by adding an admin function that allows the owner to remove games that are no longer needed. Additionally, consider adding a limit on the number of games that can be created by a single user. You could also track the active entries in the games mapping and archive them when they are no longer needed (maybe off-chain).

Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational

Code suggestions or observations that do not pose a direct security risk.

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational

Code suggestions or observations that do not pose a direct security risk.

Support

FAQs

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