reentracy attack on [burnToMint](https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L258) function

[c4-submissions]

Lines of code

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

Vulnerability details

Vulnerability details

• any user can burn one token and mint another if burnToMintCollections[_burnCollectionID][_mintCollectionID] set to true. by calling the function burnToMint from NextGenMinterContract contract.
• this function calls burnToMint function from NextGenCore contract:

   function burnToMint(
        uint256 mintIndex,
        uint256 _burnCollectionID,
        uint256 _tokenId,
        uint256 _mintCollectionID,
        uint256 _saltfun_o,
        address burner
    ) external {
        require(msg.sender == minterContract, "Caller is not the Minter Contract");
        require(_isApprovedOrOwner(burner, _tokenId), "ERC721: caller is not token owner or approved");
        collectionAdditionalData[_mintCollectionID].collectionCirculationSupply += 1;
        if (
            collectionAdditionalData[_mintCollectionID].collectionTotalSupply
                >= collectionAdditionalData[_mintCollectionID].collectionCirculationSupply
        ) {
            //@audit-issue : reentrancy .
            _mintProcessing(mintIndex, ownerOf(_tokenId), tokenData[_tokenId], _mintCollectionID, _saltfun_o);
            _burn(_tokenId);
            burnAmount[_burnCollectionID] = burnAmount[_burnCollectionID] + 1;
        }
    }

• as you see the function mint the user a token from the _mintCollectionID before burning the token by calling _mintProcessing() function .
• the _mintProcessing() function uses _safeMint() to mint the token. which make an external call to the _recipient if it's a contract's.

  function _mintProcessing(
        uint256 _mintIndex,
        address _recipient,
        string memory _tokenData,
        uint256 _collectionID,
        uint256 _saltfun_o
    ) internal {
        tokenData[_mintIndex] = _tokenData;
        collectionAdditionalData[_collectionID].randomizer.calculateTokenHash(_collectionID, _mintIndex, _saltfun_o);
        tokenIdsToCollectionIds[_mintIndex] = _collectionID;
  >>        _safeMint(_recipient, _mintIndex);
    }
    // safe mint function :
    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"
        );
    }

    // _checkOnERC721Received function that calles the to address :
    function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes memory data)
        private
        returns (bool)
    {
        if (to.isContract()) {
   >>         try IERC721Receiver(to).onERC721Received(_msgSender(), 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 {
                    /// @solidity memory-safe-assembly
                    assembly {
                        revert(add(32, reason), mload(reason))
                    }
                }
            }
        } else {
            return true;
        }
    }

• in this case a malicious _recipient will be able to reenter the contract by using a contract that calls burnToMint
  each time it's onERC721Received() function get called from the NextGenCore contract with the same token to be burned before it get burned . and mint another token from the same collection with each call.

impact

1. the user can mint all the tokens avaiable of a collection while burning only one token. by keep calling mintToBurn each time the contract calls onERC721Received() .
2. also the collectionTotalAmount[_mintCollectionID] = collectionTotalAmount[_mintCollectionID] + msg.value; will be wrong . since the attacker is providing the price of token to be minted(msg.value) each time he mint a new token. while
   collectionTotalAmount[_mintCollectionID] = collectionTotalAmount[_mintCollectionID] + msg.value; get recoded only for once ofter the reentrency ends.

tool used

vs code
manual review

Recommended Mitigation Steps

• user ReentrancyGuard from openzeppelin.
• use Checks Effects Interactions pattern. only mint token after burning the token and update the state.

Assessed type

Reentrancy

[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