Rock Paper Scissors

Summary

The contract assumes that winningToken.transferFrom(...) will succeed, but does not check its return value. This violates the ERC-20 standard and can lead to silent failures, especially when dealing with non-compliant or malicious tokens.

As a result, a player may be able to join a game or create one without actually transferring the required token, leading to a broken game state and potential denial of service or economic imbalance.

Vulnerability Details

Location

File: RockPaperScissors.sol

Root Cause

The ERC-20 standard defines transferFrom(...) as a function that returns a bool indicating success or failure. Ignoring that return value opens the contract up to:

• Silent failure (if the token didn't transfer)

• Logical errors (a game starts or is joined without the token being held)

• Inconsistencies in game state (e.g., attacker didn't stake a token but still participates)

This is especially critical when the protocol allows external or upgradeable tokens, or if WinningToken is ever modified to be pausable or governed.

Impact

• Players may join or create a game without actually staking the required token

• Results in an inconsistent or invalid game state

• May allow griefing (player enters games without committing value)

• Could block payout flows or mislead honest players

While WinningToken appears to be custom and compliant, this pattern is dangerous in general, and has been the source of bugs in real-world DeFi projects.

Tools Used

• Slither (unchecked-transfer)

• Manual inspection

• Review of ERC-20 token flow

Recommendations

Fix 1: Check the Return Value
Update both function calls to:

This ensure the token transfer must succeed or the transaction reverts.

Fix 2 (Optional): Use SafeETC20 for Robust Handling

This wraps the transfer in a safe call that checks return values and handles non-standard token implementations safely.