MyCut

Summary

This findings report highlights the critical issue found in the smart contracts ContestManager.sol and Pot.sol. The issue is related to the unchecked return values of transferFrom and transfer methods from ERC20 token transfers. It underscores the importance of checking the return values of these transactions to ensure they succeed, preventing potential vulnerabilities.

Vulnerability Details

• Locations in Code:

  • ContestManager.sol: 40

    token.transferFrom(msg.sender, address(pot), totalRewards);

    ​

    • Source: src/ContestManager.sol#31-41

  • Pot.sol: 59

    i_token.transfer(msg.sender, managerCut);

    ​

    • Source: src/Pot.sol#51-67

  • Pot.sol: 70

    i_token.transfer(player, reward);

    ​

    • Source: src/Pot.sol#69-71

The return values of these ERC20 transfer operations are ignored. Several tokens do not revert on failure and instead return false. This can lead to scenarios where the transfer fails silently, potentially allowing an attacker to exploit this oversight.

Impact

Failing to check the return values of ERC20 transferFrom and transfer calls can result in undetected failed transfers. Such vulnerabilities may:

• Allow attackers to bypass transfer validation.

• Cause funds to be inaccurately managed within the contracts.

• Lead to potential financial loss and incorrect state within the smart contract, affecting trust and functionality.

Tools Used

• Manual Code Review

Recommendations

• Use OpenZeppelin’s SafeERC20 Library: Replace direct calls to transferFrom and transfer methods with OpenZeppelin's SafeERC20 library methods to ensure return values are checked and reverts are handled correctly for transfer failures.

Here is an example of the refactored code using SafeERC20:

1. Import SafeERC20:

   import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

   ​

   Kopier kode

2. Use SafeERC20 in Contract:

   using SafeERC20 for IERC20;

   ​

   Kopier kode

3. Replace Unsafe Transfers:

   • ContestManager.sol:

     token.safeTransferFrom(msg.sender, address(pot), totalRewards);

     ​

     Kopier kode

   • Pot.sol:

     i_token.safeTransfer(msg.sender, managerCut);

     ​

     Kopier kode

     i_token.safeTransfer(player, reward);

     ​

     Kopier kode

By implementing SafeERC20, you guarantee that:

• Each transfer operation is checked for success.

• The contract reverts in case of failed transfers, protecting the contract's logic and funds.

This change can help mitigate the described vulnerabilities, ensuring safer and more reliable smart contract operations.