sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

Shaheen - `mintBatch()` Allows Users To Mint Unlimited NFTs by Only Paying Fee of 1 NFT's Mint #328

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Shaheen

high

mintBatch() Allows Users To Mint Unlimited NFTs by Only Paying Fee of 1 NFT's Mint

Summary

Users Can Mint Multiple NFTs With Only Paying 1 NFT's Mint Fee using the mintBatch(address[] calldata receivers_, uint256 tokenId_, uint256 amount_, bytes calldata data_) function.

Vulnerability Detail

There are tow mintBatch functions, we are only conmcerned here about the one which allows a token's minting to multiple receivers:

    function mintBatch(address[] calldata receivers_, uint256 tokenId_, uint256 amount_, bytes calldata data_) external payable {
        ///@audit-issue H Looks like users can use this to mint multiple nfts for almost free - PoC Worked!
        FEE_MANAGER.collectMintFee{value: msg.value}(this, tokenId_, amount_, msg.sender, address(0), works[tokenId_].strategy);
        for (uint256 i = 0; i < receivers_.length; i++) {
            _issue(receivers_[i], tokenId_, amount_, data_);
        }
        _refundExcess();
    }

This function takes an array of receivers_ as an input and mint an NFT to all the addresses given in the array input. The mintBatch function first calls the collectMintFee() function of the FEE_MANAGER contract, which basically calculates NFT's minting fee and takes the fee from the user's account.

Issue

The problem is that, that the mintBatch calls the collectMintFee() function with the user inputted amount_ variable and it is expected that the users will input the correct amount_ number. Which allows users to bypass fee for multiple NFTs minting. A malicious user can come to mint 5 NFTs to multiple receivers but only give 1 input for amount_. Now, he will pay minting fee only for 1 NFT instead of paying fees for 5 mints.

An attacker exploit this vulnerabilty to mint unlimited NFTs to multiple addresses or even same address as show in the PoC:

Proof-of-Concept

    function test_batchMint_MultipleNFTsMintingWithOneMintFee_PoC() public {
        address[] memory recievers = new address[](5);
        recievers[0] = address(1);
        recievers[1] = address(1);
        recievers[2] = address(1);
        recievers[3] = address(2);
        recievers[4] = address(5);

        edition.mintBatch{value: 0.0106 ether}(recievers, 1, 1, new bytes(0));  ///> Only 1 NFT Mint Fee
        assertEq(edition.totalSupply(1), 5);                                    ///> 5 NFTs Minted on 3 Multiple Addresses
    }

Impact

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L304

Tool used

Claws

Recommendation

I would suffest a team to find the best solution to this problem, but I guess the solution for this will be to not take amount_ from the users and use receivers_.length instead:

    function mintBatch(
        address[] calldata receivers_,
        uint256 tokenId_,
-        uint256 amount_,
        bytes calldata data_
    ) external payable {
        // wake-disable-next-line reentrancy
-        FEE_MANAGER.collectMintFee{value: msg.value}(this, tokenId_, amount_, msg.sender, address(0), works[tokenId_].strategy);
+        FEE_MANAGER.collectMintFee{value: msg.value}(this, tokenId_, receivers_.length, msg.sender, address(0), works[tokenId_].strategy);
        for (uint256 i = 0; i < receivers_.length; i++) {
            _issue(receivers_[i], tokenId_, amount_, data_);
        }

        _refundExcess();
    }

Duplicate of #264