Re-entry vulnerability in the burnToMint function

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L213-L232
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/ERC721.sol#L407

Vulnerability details

Impact

When executing the burnToMint function, an attacker can reenter so that only one NFT is burned and thus mint multiple NFTs.

Since the check of the _isApprovedOrOwner function is at L215, the _mintProcessing function is at L218, and the _burn function is at L219, the user reenters the call to this function via the _checkOnERC721Received function in the _mintProcessing function, and the NFTs are still not yet burn. The NFT is still not burn and the user can continue to mint.

Finally, the same NFT is burned multiple times, but does not generate an error, so it is able to reenter successfully.

Proof of Concept

1. in the ownerOf(_tokenId) contract, the reentrant code has been deployed, and in the onERC721Received function, the reentrant calls MinterContract::burnToMint several times.
2. the user calls MinterContract::burnToMint normally.
3. trigger reentrant code, mint multiple NFTs.
4. Execute the destruction logic, _burn multiple times but without any error reporting.

    function _burn(uint256 tokenId) internal virtual {
        address owner = ERC721.ownerOf(tokenId);

        _beforeTokenTransfer(owner, address(0), tokenId, 1);

        // Update ownership in case tokenId was transferred by `_beforeTokenTransfer` hook
        owner = ERC721.ownerOf(tokenId);

        // Clear approvals
        delete _tokenApprovals[tokenId];

        unchecked {
            // Cannot overflow, as that would require more tokens to be burned/transferred
            // out than the owner initially received through minting and transferring in.
            _balances[owner] -= 1;
        }
        delete _owners[tokenId];

        emit Transfer(owner, address(0), tokenId);

        _afterTokenTransfer(owner, address(0), tokenId, 1);
    }

Tools Used

manual

Recommended Mitigation Steps

It is recommended that burn be executed before mint

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #1198

[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 #90

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid