MyCut

First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Invalid

Use of transfer(player, reward) instead of safeTransfer() may cause token transfer failures

Summary

The claimCut() and closePot() functions use the i_token.transfer() method to distribute rewards, which may fail silently for some tokens that return false on failure. This could lead to rewards not being transferred properly to claimants or the manager without any indication of failure.

Vulnerability Details

In the current implementation, the _transferReward() function is responsible for transferring rewards to claimants and the manager. The function uses i_token.transfer() to handle token transfers:

function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward);
}

However, some ERC20 tokens do not revert on transfer failure and instead return false, which transfer() does not handle properly. Additionally, some tokens impose transfer taxes or fees, meaning the recipient might receive less than expected. This could lead to discrepancies between the intended and actual reward distribution, causing the following issues:

  1. Silent Transfer Failures: For tokens that return false on failure, the transaction will continue without reverting, potentially leaving rewards untransferred without the contract being aware.

  2. Taxed or Funky Tokens: Tokens that deduct transfer fees might cause the reward sent to the recipient to be less than intended, resulting in incorrect reward distribution or a mismatch between the remaining balance and expected rewards.

Example Scenario:

  1. Initial Setup:

    • A pot is created with 1000 tokens in total rewards.

    • Five claimants are eligible to receive rewards.

  2. Scenario:

    • The claimCut() function is called to transfer rewards to a claimant.

    • The token used by the contract has a transfer fee of 2%. When 100 tokens are transferred, only 98 tokens actually reach the claimant.

    • The contract does not account for this discrepancy, and the remainingRewards variable becomes inconsistent with the actual token balance, leading to potential further issues during future claims or when closing the pot.

Impact

  1. Silent Failures: If a transfer fails (e.g., due to insufficient balance or other reasons), the function will not revert, leaving the rewards unpaid while the contract continues execution as if the transfer was successful.

  2. Incorrect Reward Distribution: If a token imposes transfer fees or taxes, the reward sent to claimants may be less than intended, causing discrepancies in remaining rewards and token balances.

Tools Used

Manual Review

Recommendations

To mitigate these risks, use safeTransfer() from the OpenZeppelin SafeERC20 library, which ensures the transaction will revert if the transfer fails.

Steps to Implement:

Import the SafeERC20 Library: Import the SafeERC20 utility and apply it to the IERC20 token interface:

import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
using SafeERC20 for IERC20;
  • Replace transfer() with safeTransfer(): Modify the _transferReward() function as follows:

function _transferReward(address player, uint256 reward) internal {
i_token.safeTransfer(player, reward);
}
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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