Unchecked Token Transfer

Overview

• Severity: High
• Confidence: Medium
• Affected Versions: All

What is the Unchecked Token Transfer vulnerability?

In Ethereum, the ERC-20 standard is commonly implemented for fungible tokens, which are often used for decentralized finance (DeFi) projects. The standard specifies a core method, transfer(), which per the specification must return a Boolean value indicating whether or not the transfer succeeded. While the specification indicates that transfers that would fail due to insufficient funds should throw an error, this is not a strict requirement. If DeFi or other projects perform token transfers in this manner without checking this return value, they may silently fail to send tokens to intended recipients.

Further reading: ERC-20 Token Standard

Technical example of vulnerable code

For this example, let us consider two separate files. First, we have a file defining an ERC-20 token that does not throw an error on transfers that would fail:

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract SimpleERC20Token {
      mapping(address => uint256) public balances;
      mapping(address => mapping(address => uint256)) public allowed;

      function transfer(address _to, uint256 _amount) public returns (bool success) {
          if (balances[msg.sender] < _amount) return false;

          balances[msg.sender] -= _amount;
          balances[_to] += _amount;
          return true;
      }

      function transferFrom(address _from, address _to, uint256 _amount) public returns (bool success) {
          if (allowed[_from][msg.sender] < _amount || balances[_from] < _amount) return false;

          balances[_from] -= _amount;
          allowed[_from][msg.sender] -= _amount;
          balances[_to] += _amount;
          return true;
      }

      // other required ERC-20 functions and initial token allocation functionality elided for brevity
  }

Separately, we have a file defining an interface for the ERC-20 token and a marketplace smart contract that wants to interact with ERC-20 tokens that conform to the interface:

  // SPDX-License-Identifier: MIT
  pragma solidity ^0.8.0;

  interface IERC20 {
      function transfer(address _to, uint256 _amount) external returns (bool);
      function transferFrom(address _from, address _to, uint256 _amount) external returns (bool);
  }

  contract Marketplace {
      IERC20 public token;
      address public owner;

      // an abstract item for sale; this could be something like an NFT
      struct Item {
          uint256 price;
          address seller;
          bool available;
      }

      mapping(uint256 => Item) public items;
      uint256 public itemCount;

      event ItemListed(uint256 indexed itemId, address indexed seller, uint256 price);
      event ItemSold(uint256 indexed itemId, address indexed buyer, uint256 price);

      constructor(address _tokenAddress) {
          token = IERC20(_tokenAddress);
          owner = msg.sender;
      }

      function listItem(uint256 _price) public {
          require(_price > 0, "Price must be greater than zero");

          itemCount++;
          items[itemCount] = Item(_price, msg.sender, true);

          emit ItemListed(itemCount, msg.sender, _price);
      }

      function buyItem(uint256 _itemId) public {
          Item storage item = items[_itemId];
          require(item.available, "Item not available");

          // because the return value is not checked here, we will not know if the payment failed!
          token.transferFrom(msg.sender, item.seller, item.price);

          item.available = false;
          emit ItemSold(_itemId, msg.sender, item.price);
      }

      // additional functionality here
  }

In this example, we have a notional marketplace for some abstract Items that can be listed for sale and purchased by interested parties. In practice, these might be NFTs or some other digital asset, but we elide these details for simplicity here. There is a flaw in the buyItem() function in the Marketplace contract here, as it does not check the return value of transferFrom() (which is used to pay for the Item in question), and thus some sellers may not actually receive payment.

Technical example of how to fix the vulnerability

  // SPDX-License-Identifier: MIT
  pragma solidity ^0.8.0;

  interface IERC20 {
      function transfer(address _to, uint256 _amount) external returns (bool);
      function transferFrom(address _from, address _to, uint256 _amount) external returns (bool);
  }

  contract Marketplace {
      IERC20 public token;
      address public owner;

      // an abstract item for sale; this could be something like an NFT
      struct Item {
          uint256 price;
          address seller;
          bool available;
      }

      mapping(uint256 => Item) public items;
      uint256 public itemCount;

      event ItemListed(uint256 indexed itemId, address indexed seller, uint256 price);
      event ItemSold(uint256 indexed itemId, address indexed buyer, uint256 price);

      constructor(address _tokenAddress) {
          token = IERC20(_tokenAddress);
          owner = msg.sender;
      }

      function listItem(uint256 _price) public {
          require(_price > 0, "Price must be greater than zero");

          itemCount++;
          items[itemCount] = Item(_price, msg.sender, true);

          emit ItemListed(itemCount, msg.sender, _price);
      }

      function buyItem(uint256 _itemId) public {
          Item storage item = items[_itemId];
          require(item.available, "Item not available");

          // because the transfer may fail, we should handle the return value here
          require(token.transferFrom(msg.sender, item.seller, item.price), "Payment failed!");

          item.available = false;
          emit ItemSold(_itemId, msg.sender, item.price);
      }

      // additional functionality here
  }

In the corrected example, we use require() on the return value of the transferFrom() call, so that in the case that the payment itself fails, the buyItem() call will also fail.