Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing check in the burnToMint function if the price was already set #1016

Closed
c4-submissions opened this issue Nov 11, 2023 · 5 comments
Closed
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1866 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/MinterContract.sol#L256-L272

Vulnerability details

Summary

The burnToMint does not check if the price is already set with require(setMintingCosts[_collectionID] == true, "Set Minting Costs"); as the other mint functions do. Therefore, users are able to mint NFTs for free if price is not set yet.

Vulnerability Details

There are multiple functions to mint NFTs which correctly implement the check if the mint costs were already set:

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
	  require(setMintingCosts[_collectionID] == true, "Set Minting Costs");
		...
}
function burnOrSwapExternalToMint(address _erc721Collection, uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, string memory _tokenData, bytes32[] calldata merkleProof, uint256 _saltfun_o) public payable {
	  bytes32 externalCol = keccak256(abi.encodePacked(_erc721Collection,_burnCollectionID));
	  require(burnExternalToMintCollections[externalCol][_mintCollectionID] == true, "Initialize external burn");
	  require(setMintingCosts[_mintCollectionID] == true, "Set Minting Costs");
		...
}

The burnToMint function misses this check:

function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o) public payable {
    require(burnToMintCollections[_burnCollectionID][_mintCollectionID] == true, "Initialize burn");
    require(block.timestamp >= collectionPhases[_mintCollectionID].publicStartTime && block.timestamp<=collectionPhases[_mintCollectionID].publicEndTime,"No minting");
    require ((_tokenId >= gencore.viewTokensIndexMin(_burnCollectionID)) && (_tokenId <= gencore.viewTokensIndexMax(_burnCollectionID)), "col/token id error");
    // minting new token
    uint256 collectionTokenMintIndex;
    collectionTokenMintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_mintCollectionID), "No supply");
    require(msg.value >= getPrice(_mintCollectionID), "Wrong ETH");
    uint256 mintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
    // burn and mint token
    address burner = msg.sender;
    gencore.burnToMint(mintIndex, _burnCollectionID, _tokenId, _mintCollectionID, _saltfun_o, burner);
    collectionTotalAmount[_mintCollectionID] = collectionTotalAmount[_mintCollectionID] + msg.value;
}

This allows users to mint NFTs for free between the time of setting the data for the NFT drop and setting the price, as the getPrice function call will return 0:

function getPrice(uint256 _collectionId) public view returns (uint256) {
    uint tDiff;
    if (collectionPhases[_collectionId].salesOption == 3) {
        ...
    } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
				...
    } else {
        // fixed price
        return collectionPhases[_collectionId].collectionMintCost;
    }
}

salesOption is not set and therefore takes the default value 0 of uin256 and therefore getPrice will return collectionMintCost which is also not set and takes the default value 0.

Impact

Users can mint NFTs for free and therefore steal funds from the artist and the protocol.

Tools Used

Manual Review

Recommendations

Check if the price was already set.

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 11, 2023
c4-submissions added a commit that referenced this issue Nov 11, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #478

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as not a duplicate

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as duplicate of #1683

@c4-judge
Copy link

c4-judge commented Dec 4, 2023

alex-ppg marked the issue as duplicate of #1866

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Dec 7, 2023
@c4-judge
Copy link

c4-judge commented Dec 7, 2023

alex-ppg marked the issue as unsatisfactory:
Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1866 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants