Rock Paper Scissors

Summary

The RockPaperScissors contract ignores the return values from transferFrom calls when handling token transfers in createGameWithToken and joinGameWithToken functions. While many ERC20 tokens revert on failure, some tokens (like USDT) may return false instead of reverting. This could lead to games being created or joined without actual token transfers occurring.

Vulnerability Details

In both createGameWithToken and joinGameWithToken functions, the contract calls transferFrom on the WinningToken contract but does not check the return value:

According to the ERC20 standard, transferFrom returns a boolean value indicating success or failure. OpenZeppelin's implementation reverts on failure, but this behavior is not guaranteed across all ERC20 tokens, especially non-standard implementations.

While the current WinningToken implementation (based on OpenZeppelin's ERC20) reverts on failed transfers, the contract should follow defensive programming practices by checking return values, especially if there's any possibility of the token contract being upgraded or replaced in the future.

Impact

The impact is considered low because:

1. The current WinningToken implementation would revert on failed transfers

2. No immediate funds are at risk

3. The issue would only manifest if the token implementation were changed or a different token were used

However, if the token implementation were changed or if the contract were adapted to use other tokens, this could lead to:

• Games being created or joined without actual token transfers

• Inconsistent contract state versus actual token balances

• Players potentially losing tokens if the game completes but token transfers had failed

Tools Used

Slither static analysis and manual review

Recommendations

Implement proper return value checking for all token transfers. OpenZeppelin's SafeERC20 library is recommended for this purpose:

Alternatively, if SafeERC20 is not preferred, explicitly check the return values:

This ensures that the contract state is only updated when token transfers are successful, maintaining consistency between game state and token balances.