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

Functions does not check if gameID exist

dappscout

Summary

The RockPaperScissors contract fails to validate that game IDs exist before operating on them in nearly all game-related functions. This critical vulnerability allows calls to be made with non-existent game IDs, which can lead to unexpected behavior, silent failures, and in some cases security vulnerabilities due to operations on uninitialized structs.

Vulnerability Details

Throughout the contract, multiple functions take a _gameId parameter but never verify that this ID corresponds to an existing game before accessing the games[_gameId] mapping. When a non-existent game ID is provided, the contract doesn't revert but instead operates on an empty/default struct, which can have various negative consequences.

Examples of vulnerable functions include:

  1. commitMove

function commitMove(uint256 _gameId, bytes32 _commitHash) external {
Game storage game = games[_gameId]; // No validation that _gameId exists
// Function logic...
}

  1. revealMove

function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external {
Game storage game = games[_gameId]; // No validation that _gameId exists
// Function logic...
}

  1. timeoutReveal

function timeoutReveal(uint256 _gameId) external {
Game storage game = games[_gameId]; // No validation that _gameId exists
// Function logic...
}

  1. Internal functions like _determineWinner

function _determineWinner(uint256 _gameId) internal { //@audit Again, no vailidation if game exists
Game storage game = games[_gameId];
// Function logic...
}

The vulnerability extends to view functions like canTimeoutReveal and canTimeoutJoin, which return information about potentially non-existent games.

Impact

Medium to high severity. The vulnerability creates multiple issues:

  1. Silent Failures: Many operations may silently fail or produce incorrect results when operating on non-existent games.

  2. Unpredictable Behavior: Functions could behave in unexpected ways when operating on default structs, potentially causing state confusion.

  3. Gas Waste: Users might waste gas on transactions that perform meaningless operations on non-existent games.

  4. Logical Errors: Functions like timeouts and cancellations might incorrectly report success when no actual game exists.

  5. Potential for Further Exploitation: In combination with other vulnerabilities, this could be used to manipulate contract state in unintended ways.

While this issue might not directly lead to fund theft on its own, it severely undermines the contract's reliability and could be a component in more complex attacks.

Tools Used

  • Manual code review

  • Static analysis

  • Control flow analysis

Recommendations

  1. Add a helper function to validate game existence:

function _gameExists(uint256 _gameId) internal view returns (bool) {
// A game exists if it has a non-zero playerA address
return games[_gameId].playerA != address(0);
}

  1. Add validation at the beginning of each function that operates on a game:

function commitMove(uint256 _gameId, bytes32 _commitHash) external {
require(_gameExists(_gameId), "Game does not exist");
Game storage game = games[_gameId];
// Rest of the function...
}

  1. For internal functions that are always called after public functions have already validated the game exists, you can add assertions as a defensive programming measure:

function _determineWinner(uint256 _gameId) internal {
assert(_gameExists(_gameId)); // Should never fail if called correctly
Game storage game = games[_gameId];
// Rest of the function...
}

  1. Consider implementing a game registry or existence mapping to track valid game IDs.

These measures will ensure that operations are only performed on valid games, improving contract reliability and security.

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.

Support

FAQs

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