Summary
The transferFrom function, as defined in the ERC-20 standard, returns a boolean value indicating the success or failure of the token transfer. The current implementation of the createGameWithToken and joinGameWithToken functions in the RockPaperScissors contract invokes winningToken.transferFrom() but does not check the returned boolean value. This oversight can lead to scenarios where the token transfer fails (e.g., due to insufficient allowance or other token-specific restrictions), yet the contract proceeds as if the transfer was successful. This discrepancy between the intended state (tokens held by the contract) and the actual state can introduce inconsistencies and potentially break core game logic.

Vulnerability Details
Proof of Concept:

The following steps outline a scenario demonstrating the vulnerability in the createGameWithToken function. A similar approach can be used to demonstrate the issue in joinGameWithToken.

1. Prerequisites:

   • A deployed instance of the RockPaperScissors contract (RPS).

   • A deployed instance of the WinningToken contract (WT).

   • An Ethereum account acting as Player A (PA) holding a balance of WT.

   • PA has not approved RPS to spend their WT (i.e., WT.allowance(PA, RPS) == 0).

2. Execution Steps:

   • PA initiates a game by calling the RPS.createGameWithToken(1, 300) function. This transaction will be submitted to the Ethereum network.

3. Observed Behavior:

   • The require(winningToken.balanceOf(msg.sender) >= 1, "Must have winning token"); check in createGameWithToken will pass as PA holds sufficient WT.

   • The call to WT.transferFrom(PA, address(RPS), 1) will be executed. Due to the lack of prior approval, this token transfer will fail at the WinningToken contract level, and the function will return false.

   • However, the RockPaperScissors contract does not check this return value. Consequently, the execution flow will continue.

   • A new game record will be created in the games mapping within the RPS contract. The playerA field will be set to PA, and the game state will be Created.

   • The GameCreated event will be emitted, indicating a seemingly successful game creation.

   • Crucially, the RPS contract will not have received any WinningToken from PA.

Impact
The RPS contract's internal state is now inconsistent. It records a game created with a token stake, but it holds no such token.

• If another player were to join this game, the subsequent game logic, especially concerning token-based rewards, would be operating on a flawed premise.

• This silent failure could lead to unexpected behavior during game completion or cancellation, potentially resulting in incorrect token distribution or loss of funds for participants.

Tools Used
Manual Audit, aderyn, slither

Recommendations
To mitigate this vulnerability, it is imperative to check the boolean return value of the winningToken.transferFrom() function calls in both createGameWithToken and joinGameWithToken. If the transfer fails (returns false), the contract should revert the transaction to ensure data integrity and prevent inconsistent state. This can be achieved by wrapping the transferFrom calls within a conditional statement or using a require statement to assert the success of the transfer.

Example of Corrected Implementation (for createGameWithToken):
// Counter for game IDs
uint256 public gameCounter;