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

Unchecked Return Value of transferFrom May Lead to Undetected Token Transfer Failures

The Best Audit Group

Summary

A transferFrom call is made to an ERC20 token contract without verifying its return value. This can lead to unexpected behavior if the transfer fails silently (i.e., returns false but does not revert).

Vulnerability Details

ERC20’s transferFrom() function is defined to return a bool indicating success. Some non-standard or outdated tokens may return false rather than reverting on failure.

In both of these functions i.e
RockPaperScissors::createGameWithToken and RockPaperScissors::joinGameWithTokenhave the same issue

// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);

Impact

Medium.

  • Failing to check the result may cause logic to proceed as if a token was transferred, when in fact it was not.

  • This is particularly risky when interacting with non-compliant ERC20 tokens, or tokens like USDT that do not revert on failure.

Tools Used

  • Manual Review

  • Slither

  • OpenZeppelin ERC20 standard documentation

Recommendations

  • Use OpenZeppelin’s SafeERC20 wrapper which handles non-reverting tokens safely:

pragma solidity ^0.8.13;
​
import "./WinningToken.sol";
+ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+ using SafeERC20 for IERC20;
.
.
.
​
function createGameWithToken(uint256 _totalTurns, uint256 _timeoutInterval) external returns (uint256) {
​
- winningToken.transferFrom(msg.sender, address(this), 1)
+ winningToken.safeTransferFrom(msg.sender, address(this), 1);
​

• Alternatively, explicitly check the return value:

require(winningToken.transferFrom(msg.sender, address(this), 1), "Transfer failed");
Updates

Appeal created

m3dython Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Missing Check on External Call Return Value

ERC20 implementation typically reverts on transfer failures

m3dython Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Missing Check on External Call Return Value

ERC20 implementation typically reverts on transfer failures

Support

FAQs

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