Rock Paper Scissors

Summary

The createGameWithToken() and joinGameWithToken() functions relies on the ERC20 transferFrom() function to transfer tokens from the player's wallet to the contract. However, no prior approve() function call is required or checked. This causes the transferFrom() function to fail, as the contract will not have permission to transfer the player's tokens. As a result, players will not be able to start a game, making the functionality of this feature unusable.

Vulnerability Details

In thecreateGameWithToken() and joinGameWithToken() functions, the following line is used to transfer the token from the player’s wallet to the contract:

This operation relies on the player granting the contract permission to transfer the token via the ERC20 approve() function. However, the contract does not include any checks to ensure that the player has called approve() to authorize the transfer and without a prior approve() call, the allowance for the contract is 0, meaning the transferFrom() call will fail because the contract has no permission to transfer the player's tokens.

Proof Of Code

In the RockPapersScissorsTest.t.sol file, the test function does implement the approve(...) function, thus that test case passed successfully. But if we comment out the approve function in the testJoinGameWithToken(as even the actual game does not implement an approve function) then that test case does fail.

Impact

When the transferFrom() fails, the entire createGameWithToken() function will revert, and the game will not be created. As a result, no player will be able to start a game using this function without manually calling approve() beforehand.

Tools Used

Foundry

Recommendations

1. Add an Allowance Check: Before calling transferFrom(), check that the user has granted the contract sufficient allowance:

   require(winningToken.allowance(msg.sender, address(this)) >= 1, "Contract not approved to spend token");

2. Clear Error Messaging: Provide clear and actionable error messages to users so they can understand what went wrong:

   bool success = winningToken.transferFrom(msg.sender, address(this), 1);require(success, "Token transfer failed. Ensure the contract has been approved to spend your token.");