code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

MinterContract::burnToMint() do not mint any token for the case where current circulationSupply greater than eq to totalSupply, make user pay ETH for nothing. #1959

Closed c4-submissions closed 7 months ago

c4-submissions commented 7 months ago

Lines of code

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

Vulnerability details

Impact

MinterContract::burnToMint() allows burning and minting a new token in the same function. Every time a new NFT get mint, the minter has to pay a price return by getPrice(_mintCollectionID)(in ETH). The issue is if the circulation supply for that collection exceed the totalSupply in NextgenCore contract. User would end up paying eth for nothing. As no nft minted to the user.

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 = collectionAdditionalData[_mintCollectionID].collectionCirculationSupply + 1;
    // @audit below check will failed if circulation supply exceed current totalSupply

    if (collectionAdditionalData[_mintCollectionID].collectionTotalSupply >= collectionAdditionalData[_mintCollectionID].collectionCirculationSupply) {
        _mintProcessing(mintIndex, ownerOf(_tokenId), tokenData[_tokenId], _mintCollectionID, _saltfun_o);
        // burn token
        _burn(_tokenId);
        burnAmount[_burnCollectionID] = burnAmount[_burnCollectionID] + 1;
    }
}

Proof of Concept

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L270 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L217

Tools Used

Manual review

Recommended Mitigation Steps

Add an else block, which revert for the case where totalSupply reached for that collection.

Assessed type

ETH-Transfer

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #1985

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #1282

c4-judge commented 7 months ago

alex-ppg marked the issue as unsatisfactory: Insufficient proof