MinterContract.burnToMint FUNCTION DOES NOT CHECK WHETHER setMintingCosts IS SET FOR A SPECIFIC _mintCollectionID THUS ALLOWING FREE MINTS USING THE burnToMint FUNCTION

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

In both the MinterContract.mint function and the MinterContract.burnOrSwapExternalToMint functions there is a check to ensure that the minting cost of the collection is set as shown below:

    require(setMintingCosts[_mintCollectionID] == true, "Set Minting Costs");

But the same check is not performed in the MinterContract.burnToMint function which is also used to mint a collection ERC721 token after burning a token of a NextGen collection. If the setMintingCosts is not set for the specific _mintCollectionID it means the MinterContract.setCollectionCosts is not called thus the parameters such as collectionPhases[_collectionID].collectionMintCost,collectionPhases[_collectionID].collectionEndMintCost and collectionPhases[_collectionID].rate have 0 as their respective values.

As a result the getPrice(_mintCollectionID) function call inside the burnToMint function will return 0 if the setCollectionCosts is not set for the specific _mintCollectionID. As a result the following require statement will pass even for msg.value == 0.

    require(msg.value >= getPrice(_mintCollectionID), "Wrong ETH");

Hence the burnToMint function can be called by anyone (since there is no access control) to mint ERC721 tokens without paying the underlying native tokens for the mint if the setCollectionCosts is not set for the specific _mintCollectionID.

Hence any address with the approval to burn the tokenId or the owner of the burning tokenId can mint any other NextGen collection token (with specific _mintCollectionID) without paying the underlying mint cost given that the setMintingCosts is not set for that specific _mintCollectionID.

Proof of Concept

        require(setMintingCosts[_collectionID] == true, "Set Minting Costs");

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

        require(setMintingCosts[_mintCollectionID] == true, "Set Minting Costs");

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to check the setMintingCosts condition inside the MinterContract.burnToMint function by implementing the following require statement inside the burnToMint function as shown below:

function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o) public payable { 
    require(setMintingCosts[_mintCollectionID] == true, "Set Minting Costs");

The above modification ensures that the setMintingCosts is set for the specific _mintCollectionID before minting of a new tokenId takes place. If the setMintingCosts is not set the transaction will revert not allowing any user to mint the ERC721 token for free.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

a2rocket (sponsor) confirmed

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[alex-ppg]

The Warden specifies that a collection can be set up for the burnToMint execution path by a NextGen administrator via initializeBurn without having collection costs defined.

The Sponsor has confirmed this submission, however, the code does not function as the Warden describes.

The burnToMint function relies on a collection's phases having been properly set up as observed here.

To specify a collection's phases, the collection costs are mandated to have been defined. As such, this submission is invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid